I'm getting 410's trying to read garbage/dustin/foo, but 409's trying to set. Update seems to work. So, maybe the better way to phrase this is: there's no good reason to distinguish update and set in this API.
Comment on attachment 8714572 [details] [review] https://github.com/taskcluster/taskcluster-secrets/pull/12 Took a look at this and I think there's a couple of open questions. Please set the flag back to ? when I should take another look
Comment on attachment 8714572 [details] [review] https://github.com/taskcluster/taskcluster-secrets/pull/12 On the basis of the "good to go" comment, I merged PR 11 but not this one, only to realize all of your concerns were about the commits in PR 11, except the race condition. I don't think that's an issue here. Care to have another look?
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Sorry, didn't see the review flag. If you don't mind, the issues that I raised would still be good to fix.
I'm not too worried about the race. It would come if the set API call occurred at exactly the time the expiration utility deleted the old entity (which is some time *after* its expires timestamp). In that case, the user will get an error and the client will retry. That was the only issue I didn't address.
oh! yah, the race doesn't concern me too much. it was the other things, so if those are addressed, great!
Attachment #8714572 - Flags: review?(jhford) → review+
You need to log in before you can comment on or make changes to this bug.