Closed Bug 1440800 Opened 8 years ago Closed 7 years ago

Verify that *exactly* the expected context items are passed to api.setup

Categories

(Taskcluster :: Services, enhancement)

Product:
Taskcluster
Component:
Services
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: dustin, Assigned: kritisingh1.ks, Mentored)

Details

taskcluster-lib-api allows declaring expected context items. It doesn't *require* that they be declared, though. So in many cases we forget. We should be more strict about that. Also, in lots of places there's a comment somewhere in `api.js` that lists some of the context items we want, and then the `new Api(..)` has a *different* set of context items that are required. That should all get unified.
The `contexts` attribute to an API is described here; https://github.com/taskcluster/taskcluster-lib-api#declaring-apis The setup method is described here: https://github.com/taskcluster/taskcluster-lib-api#api-server-setup The first part of this bug is to check that the `context` properties for those two things match. Once that's done, and a new version of taskcluster-lib-api is released, we will need to upgrade each of the Taskcluster services to use the new version, fixing up any discrepancies in the context they provide (and also adding comments about the context elements).
Assignee: dustin → nobody
Mentor: dustin
I would like to take this up. May I ? I won't delay the PR this time. Thanks.
Sure thing!
Assignee: nobody → kritisingh1.ks
:dustin what I understand from this is that I need to add a method in api.js which will check that the list of context items declared while declaring the API is the same as that passed in api.setup. Leave some comments to let me know if I am thinking correct or not. Thanks a lot.
I don't know if you will need to add a method, but yes -- api.js is the file to patch.
:dustin here is the PR : https://github.com/taskcluster/taskcluster-lib-api/pull/84 Although I am not very confident if I have done it correct at all. Sincere apologies for that. Please review and let me know further changes.
That will be version 8.0.0 of this library. Do you want to try upgrading, say, taskcluster-secrets or taskcluster-hooks to use this version? The patch to upgrade will be simple, but we'll need to make sure that the tests all pass once that's done.
Yeah I will upgrade those repos. There are only two, namely secrets and hooks, right?
There are a bunch, but that's a good place to start and see what issues we encounter :)
We've gotten just about all of these -- I think notify and queue are still waiting for review. Nice work modifying each of those services!
Hi Dustin, is there any other taskcluster repo that needs an upgrade? If yes please let me know. Thanks :)
Nope, I think you did everything :)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Component: Platform and Services → Services
You need to log in before you can comment on or make changes to this bug.