Closed Bug 1194857 Opened 9 years ago Closed 8 years ago

Default index entry expiration to task.expires (when indexed from route)

Categories

(Taskcluster :: Services, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jonasfj, Unassigned, Mentored)

Details

(Whiteboard: [good first bug][lang=js][mentor-lang=danish])

When indexed using custom routes. We should default expiration to task.expires.
Mentor: jopsen
Whiteboard: [good first bug][lang=js][mentor-lang=danish]
Hello, I would like to work on this bug as my university assignment. Can you please give me some advice since this is my first bug?
Flags: needinfo?(jopsen)
@josef, Cool, this is a good getting started task, if you get bored afterwards let us know... bstack and I have some interesting plans for taskcluster-index making better use of NoSQL :) First get the taskcluster-index tests running locally, this is not as easy as it seems (sorry). Fill out: https://github.com/taskcluster/taskcluster-index/blob/master/user-config-example.yml And save it as user-config.yml You can get pulse credentials from: https://pulseguardian.mozilla.org/ (let me know if that's a problem) Send me your public GPG key, I'm jopsen@gmail.com, and I'll return some taskcluster credentials with scopes sufficient to run the tests. Oh, and don't be shy we're on irc.mozilla.org in #taskcluster come say hi, @bstack, is there a role containing all the scopes necessary to run the tests? I'm hoping I can just create a set of credentials for josef with that role, and then it'll just work.
Flags: needinfo?(jopsen) → needinfo?(bstack)
@josef, Also I see you used needinfo, please keep doing that when reaching me :) I'm a not good at monitoring all bugs and all email from bugs, there is just too much.
@Jonas, Hi, I've sent you the key on your mail. It might have ended in the spam folder. That's why I'm writing this way as well.
@Josef, I've emailed you some credentials for a quick user I setup: https://tools.taskcluster.net/auth/clients/#mozilla-ldap%252fjojensen@mozilla.com%252fjosef
clear ni, resolved over irc.
Flags: needinfo?(bstack)
@josef, let me know if you can get tests running and passing :)
@Jonas, I'm having some troubles running the tests. I've sent more info via mail.
Flags: needinfo?(jopsen)
Resolved via. email.
Flags: needinfo?(jopsen)
Thank you. I can get tests running and passing now.
@Jonas, I think I have the env. prepared. Can you give me some hint about the bug "Default index entry expiration to task.expires" please?
Flags: needinfo?(jopsen)
@josef, Hi, checkout this code: https://github.com/taskcluster/taskcluster-index/blob/ce4d11e2b73c2c9555e208d2dd611865edc961d8/src/handlers.js#L124-L133 Clearly, if task.extra.index.expires isn't present, then expiration will be 1 year from now. task.expires could be a sane default, see: http://docs.taskcluster.net/queue/api-docs/#task
Flags: needinfo?(jopsen)
@Jonas, Hi I think I understand but handler.js already has this functionality. Where should I check whether task.extra.index.expires is present or not then?
Flags: needinfo?(jopsen)
P.S. I'm sorry for my late reactions but I couldn't have answered earlier :(
Look at: https://github.com/taskcluster/taskcluster-index/blob/ce4d11e2b73c2c9555e208d2dd611865edc961d8/src/handlers.js#L124-L133 You might have to reference lodash for the _.defaults() functions,,, but really, it's all there. Set expires, here: https://github.com/taskcluster/taskcluster-index/blob/ce4d11e2b73c2c9555e208d2dd611865edc961d8/src/handlers.js#L125 To task.expires if task.expires is present, otherwise set it to a year as is the current default.
Flags: needinfo?(jopsen)
@Jonas, I got it. Should I sent the handlers.js file to you or should I push it using git? In the second case, what branch should I create? Do I have rights to do that?
Flags: needinfo?(jopsen)
@josef, Fork the repo on github, create a branch an make a pull request: https://help.github.com/articles/fork-a-repo/
Flags: needinfo?(jopsen)
@Jonas I made a pull request. I noticed that The Travis CI build failed because of missing credentials. I hope it's not a problem since the user-config can't by part of the project :)
Is this done now?
Flags: needinfo?(jopsen)
Looks like it's not done yet... There is a PR, but we haven't merged it, or looked at it since the issues addressed in the review was fixed. I guess we really should get this landed, I feel bad it's taken this long...
Flags: needinfo?(jopsen)
I merged this just now! Looks like it is working.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Component: Index → Services
You need to log in before you can comment on or make changes to this bug.