1439058 - index.listNamespaces API method continuationToken parameter

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

[tuhina chatterjee]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36

Steps to reproduce:

The taskcluster api for listGroup
https://docs.taskcluster.net/reference/core/taskcluster-index/references/api#list-namespaces

needs to follow the same structure as the others in https://docs.taskcluster.net/reference and it should be a GET with continuationToken in its getparams.

Refer : https://bugzilla.mozilla.org/show_bug.cgi?id=1405072#c6

Comment 1

[tuhina chatterjee]

Perhaps [1] this is this place where the api has been declared.
And I will probably need to change the assignment limit and continuation parameters[2] from the `req` object.

I will also need to edit the documentation[3].


Please let me know if I am on the right track.
Also how should I go about testing these changes ?
How will backward compatibility be ensured for the clients using the current implementation of the API





[1] https://github.com/taskcluster/taskcluster-index/blob/master/src/api.js#L163
[2] https://github.com/taskcluster/taskcluster-index/blob/master/src/api.js#L188
[3] https://github.com/taskcluster/taskcluster-docs

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Yes, you are looking in the right spot!

Good question about compatibility.  

The key here is that once we remove continuationToken and limit from the request payload, there's nothing left in that payload!  So there's no need to keep using an HTTP POST for this request anymore.  That means that, for a while at least, we can define *two* API methods that do the same thing:

index.listNamespaces - GET /namespaces/:namespace?  (allowing continuationToken / limit query arguments)
index.listNamespacesPost - POST /namespaces/:namespace? (with continuationToken / limit in the request body)

This takes advantage of a little trickery.  When an application that hasn't been updated uses this API endpoint, it will keep using the POST method, which will still work fine.  We've just changed the name of it internally.  However, when we upgrade an application to use a new version of https://github.com/taskcluster/taskcluster-client, it will see that `index.listNamespaces` uses the GET method, and thus use the new version. This happens because taskcluster-client is generated from the information in `src/api.js`.

So we're doing a sneaky re-name of the old function in a way that won't cause any problems.  I made a similar change a few weeks ago in

  https://github.com/taskcluster/taskcluster-auth/commit/b9c41cae67a7fac6756f6884350bb60af02aa3b8

Note that I copy/pasted the function body in that commit.  You can do the same, or you can factor out the common bit of both API methods into a single function.

Comment 3

[tuhina chatterjee]

Hi Dustin. I have made the changes that you suggested. I dont have access to push to any new/old branch of the repository. How should I go about getting the code reviewed ?

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

You'll need to fork the repository on github and push to your fork, then create a pull request.  Then we can review that pull request and merge it if it's OK.  Some searching on the web will find guides for forking and making pull requests, but we're happy to help in #tc-contributors too, if you get stuck.

Comment 5

[tuhina chatterjee]

Pull Request Link 

https://github.com/taskcluster/taskcluster-index/pull/39

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Merged, and this seems to be working!