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)
Taskcluster
Services
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.
Reporter | ||
Updated•9 years ago
|
Mentor: jopsen
Whiteboard: [good first bug][lang=js][mentor-lang=danish]
Comment 1•9 years ago
|
||
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?
Updated•9 years ago
|
Flags: needinfo?(jopsen)
Reporter | ||
Comment 2•9 years ago
|
||
@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)
Reporter | ||
Comment 3•9 years ago
|
||
@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.
Comment 4•9 years ago
|
||
@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.
Reporter | ||
Comment 5•9 years ago
|
||
@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
Reporter | ||
Comment 7•9 years ago
|
||
@josef, let me know if you can get tests running and passing :)
Comment 8•9 years ago
|
||
@Jonas, I'm having some troubles running the tests. I've sent more info via mail.
Flags: needinfo?(jopsen)
Comment 10•9 years ago
|
||
Thank you. I can get tests running and passing now.
Comment 11•9 years ago
|
||
@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)
Reporter | ||
Comment 12•9 years ago
|
||
@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)
Comment 13•9 years ago
|
||
@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)
Comment 14•9 years ago
|
||
P.S. I'm sorry for my late reactions but I couldn't have answered earlier :(
Reporter | ||
Comment 15•9 years ago
|
||
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)
Comment 16•9 years ago
|
||
@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)
Reporter | ||
Comment 17•9 years ago
|
||
@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)
Comment 18•9 years ago
|
||
@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 :)
Reporter | ||
Comment 20•8 years ago
|
||
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)
Comment 21•8 years ago
|
||
I merged this just now! Looks like it is working.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: Index → Services
You need to log in
before you can comment on or make changes to this bug.
Description
•