1222982 - API docs should show API stability

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

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

For example, in the meta data, cancelTask is "experimental", but there is no visual indication of this in the API docs:

http://docs.taskcluster.net/queue/api-docs/#cancelTask

Similarly, importClients is "deprecated" in meta data, but this can not be seen from API docs:

http://docs.taskcluster.net/auth/api-docs/#importClients

It might be also be nice to set a background shade for the API function name in the function list at the top of each API page, e.g. grey for deprecated, and maybe something else for experimental. A similar visual cue could be given in the body of the function documentation too, as well as explicitly stating "deprecated" or "experimental".

Comment 1

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

Maybe we should just get rid of API stability -- I'm not sure it's useful information..

Comment 2

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

I kind of like it, as long as we maintain it. I think if we start showing it, we are more likely to maintain it.

Comment 3

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

Just like all of the "more later" docs have gotten filled in, right? :)

Comment 4

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

e.g., https://github.com/taskcluster/azure-entities/blob/master/README.md

Comment 5

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

I see your point.

Jonas was the driving force behind stability levels - let's see what he says. It was added less than six months ago in https://github.com/taskcluster/taskcluster-base/pull/12.

The stability levels are displayed in the go docs of the go client, e.g. see https://godoc.org/github.com/taskcluster/taskcluster-client-go/queue#Queue.CancelTask - I'm not sure if anything else currently displays them (I suspect not).

@Jonas, I'd say if you'd like to keep it, please go through the existing APIs and set appropriate stability levels, e.g. it looks like all the Queue APIs are "experimental", which doesn't make sense now we're in production. Or we say the module owner is responsible for updating the stability levels. If you agree this is fair, we can create bugs for each module, otherwise let's drop it, as dustin suggests.

Comment 6

[Jonas Finnemann Jensen (:jonasfj)]

@pmoore, dustin,
We do have API that becomes deprecated and APIs that never goes further than experimental..
aws-provisioner APIs are largely just experimental, as we someday may wish to change the concept dramatically.

I like it. and put adding stability on my todo. There is quiet a few components to go through which is
probably why I never got around to it.

Comment 7

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

I think this could be a good starter bug for someone. Note - the dependent bugs are about setting correct values for stability - this bug itself is about *showing* stability in the docs.

Comment 8

[David Král]

Hello, I would like to try this as my first bug, can you give me some info please? :)

Comment 9

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

Hi David --

I'll get you started and Pete can pick up later.

We have a bunch of services that use taskcluster-lib-api (https://github.com/taskcluster/taskcluster-lib-api) to provide REST API endpoints.  One such service is taskcluster-hooks (https://github.com/taskcluster/taskcluster-hooks), but there are many more.

In declaring an API endpoint, the service can specify a stability level, for example

https://github.com/taskcluster/taskcluster-hooks/blob/master/routes/v1.js#L159
api.declare({
  method:       'get',
  route:        '/hooks/:hookGroupId/:hookId/schedule',
  name:         'getHookSchedule',
  output:       'hook-schedule.json',
  title:        'Get hook schedule',
  stability:    'deprecated',
  description: [
    "This endpoint will return the schedule and next scheduled creation time",
    "for the given hook."
  ].join('\n')
}, ..

Taskcluster-lib-api takes all of this information and packages it into a JSON file which it publishes on https://references.taskcluster.net; for hooks the file is http://references.taskcluster.net/hooks/v1/api.json

The documentation site, http://docs.taskcluster.net, contains JavaScript code which, in the browser, loads those JSON files and displays them to the user.  For hooks, the page is http://docs.taskcluster.net/services/hooks/.

You'll notice that that documentation page does not mention API stability levels, even though they are included in the JSON file.  That's what you need to fix.

Comment 10

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

The documentation is in the https://github.com/taskcluster/taskcluster-docs repo.  You may want to look specifically at

https://raw.githubusercontent.com/taskcluster/taskcluster-docs/master/services/secrets/index.md
https://github.com/taskcluster/taskcluster-docs/tree/master/assets/docref

Comment 11

[David Král]

OK, thanks very much, I'll try to look at it.

Comment 12

[David Král]

Hello, can somebody give me an advice how to run the pages locally? I cloned the repository, but the default.html seems to be written in Django template lang that i have never seen before. Thanks :)

Comment 13

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

David, see https://github.com/taskcluster/taskcluster-docs#development-setup

Comment 14

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

@dustin, thanks for the detailed overview

Comment 15

[David Král]

Ok, now its running :) I added:  <dt>Stability</dt>
                                 <dd><code><%= entry.stability%></code></dd>

Do I need to add some styles depending on the value of stability? Or test if stability exists?

Comment 16

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

Hi David, thanks for your work!

(In reply to David Král from comment #15)
> Ok, now its running :) I added:  <dt>Stability</dt>
>                                  <dd><code><%= entry.stability%></code></dd>
> 
> Do I need to add some styles depending on the value of stability?

That would be awesome, yes please! See comment 0 for some ideas how we could exploit this, feel free to play around with styles to see what you think looks best.

> Or test if stability exists?

From http://schemas.taskcluster.net/base/v1/api-reference.json it looks like stability should be always present, and that currently its value should be one of:

  * "deprecated"
  * "experimental"
  * "stable"

However it would be best if the UI can gracefully handle other values being shown, or if no value is present, as e.g. this schema could change, and e.g. updating the docs site can easily be overlooked. Or maybe some API manages to creep in which somehow doesn't comply with the schema. Just a safeguard really, so that we don't suddenly end up with a broken docs site because something was overlooked.

Thanks again for your help!

Comment 17

[David Král]

Attachment: diff file for reference.ejs

Stability added in reference.ejs

Comment 18

[David Král]

Attachment: stability.css

I created some styles for all three stability values. I warn that I am not very good in graphic design :))

Comment 19

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

Comment on attachment 8731326 [details] [diff] [review]
diff file for reference.ejs

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

I'm not sure what the best approach is for styling these results -- I think there may be some bootstrap-y way to do this, rather than writing raw color values.  Perhaps Jonas knows more about that?

Otherwise, this looks good.  Just some formal issues to fix.

Please include both this change and the new css file in the same patch.  Also, note that you have lines with spaces at the end of the line -- Bugzilla highlights these in red.  Please remove such "trailing whitespace" from your patch.

::: assets/docref/reference.ejs
@@ +48,5 @@
>        var signature = entry.name + "(" + args.join(', ') + ") : " + retval;
> +      var stabilityValue = "unknown";
> +      if (entry.stability) {
> +        stabilityValue = entry.stability;
> +      }		

This stanza can be shortened to

 var stabilityValue = entry.stability || "unknown";

Comment 20

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

Actually, maybe Pete knows how best to handle the styles.

Comment 21

[David Král]

Attachment: patch.diff

Comment 22

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

Thanks for the patch David. The trailing space still needs to be removed from assets/docref/stability.css.

When I deploy locally, I don't see different styles for the different stability levels, I'm not sure why not. Can you confirm you have different styles when you run it?

Thanks,
Pete

Comment 23

[David Král]

Because I dont know where to link css file. Should it be in default.html or somewhere else?

Comment 24

[Jonas Finnemann Jensen (:jonasfj)]

@david,
Yeah, why not add it here:
https://github.com/taskcluster/taskcluster-docs/blob/de259ee88801f82a9a4a760a8d7d6fc7878ebc60/_layouts/default.html#L114

It's not as if we could make this file any more ugly than it already is :)
Wow, I had completely forgotten how bad that code is... Anyways, for now better docs is better.
 - We can clean up this mess some rainy day (/me lives in SF, it doesn't rain there -- hi hi)

Comment 25

[David Král]

Attachment: patch.diff

Comment 26

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

Comment on attachment 8734294 [details] [diff] [review]
patch.diff

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

I like this, thanks for working on this David!

I'd also like to add a couple of extra changes, so I'll r+ this first, and then move the further changes into a PR. Your changes will land when that PR lands.

Thanks again!

Comment 27

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

Attachment: Github Pull Request for taskcluster-docs

Comment 28

[[github robot]]

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

https://github.com/taskcluster/taskcluster-docs/commit/b1973e8508d0193e3fa02d832319b96be2eaa02a
Bug 1222982 - Add stability levels to API docs

https://github.com/taskcluster/taskcluster-docs/commit/cb960e73804551449fd753a257d35ac8f6ac6ad7
Merge pull request #88 from taskcluster/stability

Bug 1222982 - API docs should show API stability

Comment 29

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

Comment on attachment 8737217 [details] [review]
Github Pull Request for taskcluster-docs

Carrying over r+ from dustin from PR comments.

Comment 30

[David Král]

Ok, thanks for your help too :)