1460019 - Include a version in api.json

Status: RESOLVED FIXED

(Taskcluster :: Services, enhancement, P5)

Description

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

For the moment we will assume version: 0 to mean version: 'v1', but let's change it so we don't need to do that translation.

Comment 1

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

At the same time, let's change the API `name` proeprty to the more descriptive `serviceName`.

Comment 2

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

Update: version is the version of the reference format; not the version of the API.  So maybe add an apiVersion field.

Comment 3

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

Note that this is needed in both exchanges and API definitions

Comment 4

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

We'll also want to remove baseUrl from the reference format.

Comment 5

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

Once https://github.com/taskcluster/taskcluster-references/pull/4 lands we can add a new version with this information.

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: taskcluster-lib-api patch to include api version

This is the patch required to get api version into the api references

Comment 7

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: taskcluster-references patch

This is the patch that I believe should add the apiVersion field to the lib-api schema.  It's not clear to me why the schema for lib-api isn't stored in lib-api and published like we do for other schemas on deployment, what's the reason for this decision?

Comment 8

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

Comment on attachment 9009115 [details] [diff] [review]
taskcluster-lib-api patch to include api version

Review of attachment 9009115 [details] [diff] [review]:
-----------------------------------------------------------------

::: package.json
@@ +1,3 @@
>  {
>    "name": "taskcluster-lib-api",
> +  "version": "12.5.0",

This typically gets bumped in a commit of its own, so it's easy to spot and has a tag on that revision.

::: src/builder.js
@@ +181,4 @@
>    reference() {
>      const reference = {
>        version:            0,
> +      apiVersion:         this.version,

Note that this is not an integer :)

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: taskcluster-lib-api patch to update schema

Comment 10

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

Comment on attachment 9009117 [details] [diff] [review]
taskcluster-references patch

Review of attachment 9009117 [details] [diff] [review]:
-----------------------------------------------------------------

Also, I wonder if this should be api-reference-v1.yml, so that a reference document can declare that it provides this information.  Changing the schema at a particular URL seems like a recipe for making existing validations fail. On the other hand, this is a backward-compatible change.

::: schemas/api-reference-v0.yml
@@ +84,5 @@
>      enum:
>        - 0
> +  apiVersion:
> +    description: Version of the API
> +    type: integer

We've been careful to treat API versions as strings with a "v" prefix, to help disambiguate them from other things (like the confusing `version` property).  So this should be a string with a pattern, similar to that in `manifest-v2.json`.

Comment 11

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

Oh, and I think the copy of that schema in taskcluster-lib-api will need an update?

Maybe we should just hold on until we talk next week.

Comment 12

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

Comment on attachment 9009117 [details] [diff] [review]
taskcluster-references patch

I think bugzilla got confused about which patch I was reviewing..

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: taskcluster-lib-api patch to include api version (v2)

Now that I was able to find the schema (thanks!), I've been able to get the tests working

Comment 14

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

Comment on attachment 9009150 [details] [diff] [review]
taskcluster-lib-api patch to include api version (v2)

Review of attachment 9009150 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but without the version bump in this rev.  And let's hold off landing until we talk on Monday.

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: taskcluster-references patch (v2)

Comment 16

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

Comment on attachment 9009151 [details] [diff] [review]
taskcluster-references patch (v2)

Let's figure out if this should be api-reference-v1.yml before r+'ing

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #14)
> Comment on attachment 9009150 [details] [diff] [review]
> taskcluster-lib-api patch to include api version (v2)
> 
> Review of attachment 9009150 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, but without the version bump in this rev.  And let's hold off
> landing until we talk on Monday.

Do you mean that this should be a patch release or that we should build up more changes before determining the next version number?

Comment 18

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

I think a minor rev is good -- but let's make a separate commit to bump the version number with a tag, like `npm version` does.

Comment 19

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

I think this is ready to go.  I'll try to get it landed and released.

Comment 20

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

https://github.com/taskcluster/taskcluster-lib-api/pull/119
https://github.com/taskcluster/taskcluster-references/pull/7

Comment 21

[[github robot]]

Commits pushed to master at https://github.com/taskcluster/taskcluster-lib-api

https://github.com/taskcluster/taskcluster-lib-api/commit/a41a627be901c28a3c12b6607b2a254f2c6dd23a
Bug 1460019 - Include a version in api.json

https://github.com/taskcluster/taskcluster-lib-api/commit/23280b754f3f208714873e79c31a2505c46057a6
Merge pull request #119 from djmitche/bug1460019

Bug 1460019 - Include a version in api.json

Comment 22

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

tc-lib-api v12.5.0

Comment 23

[Pete Moore [:pmoore][:pete]]

This needs to be published to http://schemas.taskcluster.net/base/v1/api-reference.json

Comment 24

[Pete Moore [:pmoore][:pete]]

Dustin, a gentle remember not to close bugs for changes to api-reference.json until the changes have been manually published to production, or to deploy the changes to production when you close the bug (due to bug 1433672). Thanks!

Comment 25

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

done.