[checks] Make Checks API work with decision tasks in taskcluster-github
Categories
(Taskcluster :: Services, enhancement)
Tracking
(Not tracked)
People
(Reporter: owlish, Unassigned)
References
Details
Currently, when tc-gh is switched to the Checks API mode, it doesn't report status for the tasks that are started by a decision task.
The reason being, the handler are bound to the RabbitMQ queues with a special task-specific route (in routes
array in the task definition) - decision task would have that special route, but the tasks that it triggers will not, unless the user adds them to the task definitions by hand.
The idea of using primary key for bindings has been ruled out. Alternative solution is being worked on in Bug 1252144
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
I'll note that it is a fairly simple change to android components to enable checks.
Reporter | ||
Comment 2•6 years ago
|
||
It's not just about Android components tho. There are other projects out there too. And even Android components: today they have something simple, tomorrow they have something complicated. There needs to be some solution that makes taskcluster easier to use.
How is your bug going? Some other work does need to be done in Checks, so I will occupy myself with that in the meanwhile. Let me know if I can help with your taskgraph project
Comment 3•6 years ago
|
||
(In reply to [:owlish] from comment #2)
It's not just about Android components tho. There are other projects out there too. And even Android components: today they have something simple, tomorrow they have something complicated.
Certainly it is not just about android components. However, every implementation of a decision task that I have seen has some sort of abstraction of creating tasks, like android components does. And if that is the case, adding support for checks likely likely requires a one or two line change to that abstraction, in addition to the change to .taskcluster.yml
.
Reporter | ||
Comment 4•6 years ago
|
||
Using this or that Github API is a taskcluster-github's setting. I am not sure why do we need to edit task definitions (even if just a single abstraction) to enable certain taskcluster-github settings.
Not to mention that it would be ideal if people didn't have to change task definitions (in fact, anything) whenever we change details of Checks implementation.
Reporter | ||
Updated•5 years ago
|
Comment 5•5 years ago
|
||
Note that we are using checks in production on reference-browser with a decision task. It would be nice if we could submit checks from cron-tasks as well, though.
Reporter | ||
Comment 6•5 years ago
|
||
By cron tasks you mean Hooks?
Comment 7•5 years ago
|
||
I mean, anything that isn't taskcluster-github. In practice, that means hooks (although the task-group that would be reporting isn't started directly by the hook).
Note that we have also talked about listening to tc-github pulse message to start taskgroups for on-push and PR tasks as well. We do this already (here) for triggering l10n automation in response to pushes on other repos, but would also give a way to abstract some of the repetition that exists between all the taskgraph-using github projects' .taskcluster.yml
s.
Comment 8•5 years ago
|
||
I tried out Checks in Servo, in hope to improve the situation of bug 1548781. We have .taskcluster.yml
creating one "decision" task, which runs a Python script which in turns creates other tasks. Only the decision task shows up in Checks, and the check run is marked as successful as soon as the decision task ends even though some other tasks may go on to fail:
- https://community-tc.services.mozilla.com/tasks/groups/csJNTRpEQ2-UcgzxjVUUjw
- https://github.com/servo/servo/runs/311665021
A bot relies on this reporting to automatically merge PRs that pass all tests, so Checks is not viable for Servo yet because of this bug.
Comment 9•5 years ago
|
||
After reading this thread more carefully I’ve realized this is likely because of the missing route. I’ve filed https://github.com/mozilla/community-tc-config/pull/129 to allow adding that route.
Adding manually is not ideal, but I’m not sure what the ideal solution would look like.
The bug description says:
The idea of using primary key for bindings has been ruled out.
Why?
Reporter | ||
Updated•5 years ago
|
Description
•