Switching to checks API in tc-gh

[owlishDeveloper]

Relevant literature: https://developer.github.com/v3/checks/
I started implementing here taskcluster/taskcluster-github#278

The above needs more work in these areas:

• Decision tasks
• Maintaining compatibility with the current status API so that the third party tools that use statuses would still work.

Decision tasks part itself has two parts:

• Displaying the tasks started by the decision task as checks (which is easy: jobHandler will create the check for the decision task, and then, as we will be getting messages from the Queue, I will check if the task that came is in our DB - if yes, I'll update the checkrun, if not - I'll create one and put it into our DB). this is already in the PR
• Retriggering tasks. So Github has "Retrigger All" and "Retrigger Check" buttons in the UI which are there whether you want them or not ✨ Pressing them will generate a webhook for us to catch. The former button is always there, and the latter button appears once the task is completed. For the above PR, I did not intend to make that button functional - the above PR is the first part of the work, and it's for displaying statuses only. For the second part of the work tho, I'll need to figure out a way of retriggering the tasks.

I propose: the "Retrigger All" button restarts, well, all using deprecated✨ rerun API of the queue. the "Retrigger Check" button will create a new taskcluster task in the same taskgroup. The checkrun status will be tied to this new task. I would like to display the inspector links for both of the tasks in that check.

• Maintaining compatibility would probably make sense for the above PR. I am not sure what it means yet tho. There can be a lot of third party tools out there, I would guess... I would need more information on how they work. What I can say so far - it seems to me if you create check suites and check runs, you would not be able to get the statuses via repos.getStatuses (I'll test this some more, of course). Also, if the user doesn't give our integration the permission to read and write checks... I'm not sure what happens - apparently they are left stateles?? I don't call repos.status.create.

So, as far as I can tell right now, the only way to maintain compatibility would be to......create statuses alongside checks?? Or we can give people opportunity to decide if they want checks or statuses - in .taskcluster.yml?

[imbstack]

Let's do some more science to figure out how checks and statuses interact. I don't think it would be bad to create both, just something we should know if we need to do or not.

As for actions, I think we should think of a nice way to have automatic "taskcluster actions" like we generate for decision tasks in gecko for fallback of repos that don't have it here?

[djmitche]

Default actions should probably be a separate RFC. Let's talk a bit more about that.

[owlishDeveloper]

Some science sounds good!

Default/automatic actions is something I'm probably not familiar with, but it sounds interesting

[owlishDeveloper]

Re: checks API vs statuses API compatibility

Results of my experiments:

• If a status indicator in the UI was created with Checks, there will be no data for Statuses and vice versa. So they are two separate things (although both result similar effect in the Github UI).
• Creating UI indicators using both APIs in succession: results in two status indicators in the UI: one from Checks API, the second from Statuses API. <--- given this, I'm not sure if simply keeping both is an option.

So the only way of dealing with compatibility I see is to have users select which API they want in .taskcluster.yml, and choose our default.

[owlishDeveloper]

If we decide to go with "let the user decide" route, I think that part would actually have to be implemented and merged first. What do you think? Then we won't be having the problem of choosing our default - it will be Statuses. People won't even notice anything (or at least that's the idea); and meanwhile, we could be sneaky and test Checks on production cluster✨ and merge changes one by one. And when It is done (-ish) we could do a ✨ Dramatic Reveal ✨ - just let people know about the feature and what to stick into their .taskcluster.yml if they want it

😏

[djmitche]

Oo, data-based decision making! So, we need to support both (and I think we can mirror Github's implication that the status API is old and boring).

So we had talked about better ways to handle feedback for checks -- rather than listen on schedulerId-based routes, we could establish some checks-specific routes and just listen for those. Those routes could either be on tasks created directly from .taskcluster.yml, or from tasks created later (such as by a decision task). In that case, we'd just want a way to turn off the automatic status reports.

So maybe something as simple as

report-status: false

at the top level of .taskcluster.yml would be enough to disable status reporting; then the checks-related routes could be used within the tasks.

[owlishDeveloper]

That makes sense to me so far! The only thing, I would change the wording of the magic line a bit.

GitHub confusingly uses both the words "status" and "checks" for two different things (the indicators in the UI and the type of API used to create them). So report-status: false sounds like the indicators won't be created in the UI - which is false; either API creates those indicators. To avoid any possible confusion, I'd propose reword it as use-checks-api: true.

[djmitche]

Maybe use-status-api: false? The config is specifically to disable the existing reporting of the taskgroup's state to the github status API. There's nothing to enable for checks -- users will just use the task routes if they want the checks.

[owlishDeveloper]

I don't think it's necessary to link user interface to the implementation details (I think most of the time users don't care whether use-checks-api: true actually enables anything or what's going on under the hood in general - not to mention that the latter can always be changed but interface must stay the same). I would imagine people who would use this magic line know what Checks are and want to use them. So that wording emphasizes more what a user wants rather than what he/she is switching from. Like, the action is rather "Switch to Checks! ✨" than "Ditch Statuses 💩".

Besides, I think something is true is considered to be simpler than something is false.

That said, use-status-api: false doesn't sound confusing to me, I think it's pretty clear in what it does, so I wouldn't insist. I think use-status-api: false is fine.

[djmitche]

I think transparency of meaning is important, since our users are building applications on our platform. Here's how I see it:

use-checks-api: false:

• PRs will have results reported in the Github UI: yes
• tasks with check-related routes will be reported as checks: yes

use-checks-api: true:

• PRs will have results reported in the Github UI: no
• tasks with check-related routes will be reported as checks: yes

That seems pretty confusing to me: I say "use this thing" and the only difference is that some other feature (reporting status of my PRs) stops working.

[owlishDeveloper]

PRs will have results reported in the Github UI: no

I think the results will be reported to the GitHub UI in either case. It's just that with checks, the Checks tab will be populated, and when you view the PR, clicking on Details will take you to that tab. Whereas with Statuses API, the Checks tab will be empty, and we ourselves supply the URL for the Details link.

This is what they both look like: the top commit here has two indicators created by the tow APIs. The one with the fish is Check API.

tasks with check-related routes will be reported as checks: yes

hm, I'm not yet sure what that means, I need to think over your implementation idea to understand it better. It seems like you're saying both APIs should be used at the same time?

The way I was imagining it, was we use ye olde code by default (i.e. Statuses API), and that doesn't require any magic lines anywhere. Only if you put use-github-checks: true in .taskcluster.yml, tc-gh will switch to the new code path (i.e. Checks API). Mind you, I don't know how to implement that yet, I haven't thought about it. I don't even know if that's possible or practical. I just thought: wouldn't that be awesome if it could be done :)

[owlishDeveloper]

...and here is an example of PR with indicators* created by both APIs as simultaneously as javascript can get. The one with the fish is produced by Checks API. So both APIs create indicators!

*by "indicator", I mean yellow dots, green checkmarks, the contents of the Checks tab - all the things that display the Status or a Check to the user. Lemme know if you meant something else by "results reported in the Github UI".

[djmitche]

Yes, "indicators" is what I meant.

My comments are assuming that we implement checks using routes that are added to tasks. That doesn't require any configuration to "enable" and in fact would be sort of difficult to enable or disable. If tc-github gets pulse messages on those routes, it adds checks -- regardless of whether it was calling the status API for the same PR (or push or whatever). Supporting enabling/disabling that would require downloading and parsing a project's .taskcluster.yml for every such pulse event to see if the checks API was enabled.

So basically in six months the situation for our user would be this:

• If you want to show results in Github, we recommend you use pulse routes that tc-github will translate into checks
• If you do that, you will probably be annoyed by the "automatic" generation of redundant status information for tasks in .taskcluster.yml. To disable that, add blahblahblah to your .taskcluster.yml.
• If you have an older repository that hasn't been updated since we started supporting checks, it will keep working (at least until Github drops support for the status API).

[owlishDeveloper]

If you want to show results in Github

Do you mean Github is deprecating Statuses, and in 6 months there won't be any indicators in the UI shown if you call repos.createStatus?

[owlishDeveloper]

Don't we seem to be jamming together choosing the compatibility problem solution and its implementation? 🤔

As far as solving the compatibility problem, I see the following options:

• abandon the idea of implementing Checks altogether to preserve compatibility with the third party tools our users have;
• switch to Checks and give our users additional controls in the Github UI at the cost of sacrificing the compatibility with the tools that rely on the status API;
• implement both and either offer two separate GitHub apps (one with the checks, another with Statuses) or let users do the switch in .taskcluster.yml.

If we are OK with two sets of indicators (I was assuming we aren't), we have a fourth option:

• Implement and use both Checks and Statuses as you are suggesting.