1472792 - Update actions spec to allow multiple actions with the same name, take the first one

Status: RESOLVED FIXED

(Taskcluster :: General, enhancement)

Description

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

We allow multiple entries in actions.json to have the same name, but require that their contexts make them apply to disjoint sets of tasks.  This is hard when you want a specific implementation for a few tasks, and a more general implementation for all other tasks.

We have an "order" for actions already.  So let's say that the first matching task, in order, is the one executed.

Comment 1

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

Once this is done, the two in-tree retriggers can both be named 'retrigger'.

Comment 2

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

Bug 1398277 needs a special retrigger action that applies only to decision tasks, with the default retrigger applying everywhere else.

Comment 3

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb5eab2f73acb4753c341d8a46253ff6d20abe69

Comment 4

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb284bcb99652799a87b927427f35c55be5fbc1d

Comment 5

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

Spec update:
  https://github.com/taskcluster/taskcluster-docs/pull/270

Comment 6

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

https://github.com/taskcluster/taskcluster-tools/pull/543

Comment 7

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

We'll need a similar thing for treeherder, too.

/me smiles charmingly at Hassan..

Comment 8

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

Attachment: Bug 1472792: give both retrigger actions the same name;

Review commit: https://reviewboard.mozilla.org/r/255524/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255524/

Comment 9

[Brian Stack [:bstack]]

Comment on attachment 8990450 [details]
Bug 1472792: give both retrigger actions the same name;

https://reviewboard.mozilla.org/r/255524/#review262338

Comment 10

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

Huh, order isn't in the spec.  https://github.com/taskcluster/taskcluster-references/blob/master/schemas/action-schema-v1.yml

I think the in-tree stuff specifies an order for the array in actions.json.  So I'll re-configure..

Comment 11

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

https://github.com/taskcluster/taskcluster-docs/pull/270
https://github.com/taskcluster/taskcluster-tools/pull/543

updated to just use the existing order, rather than the (nonexistent) order field.

Comment 12

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

Comment on attachment 8990450 [details]
Bug 1472792: give both retrigger actions the same name;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255524/diff/1-2/

Comment 13

[Brian Stack [:bstack]]

Comment on attachment 8990450 [details]
Bug 1472792: give both retrigger actions the same name;

https://reviewboard.mozilla.org/r/255524/#review262606

Comment 14

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c6a04b45e71559ad5c8aecf6edab34be6398178

Comment 15

[[github robot]]

Commit pushed to master at https://github.com/taskcluster/taskcluster-tools

https://github.com/taskcluster/taskcluster-tools/commit/165e524561bef0ea92dc48552f89e62e21216cdf
Bug 1472792 - select the lowest-ordered action of a given name (#543)

Comment 16

[[github robot]]

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

https://github.com/taskcluster/taskcluster-docs/commit/5bfe3777581aab38c09eb2b807c6d3dc5c0d128e
Bug 1472792 - allow multiple actions with matching name

This takes advantage of the order in which actions are specified in the
array in `actions.json`.

https://github.com/taskcluster/taskcluster-docs/commit/642465a01c2a7ae9309631861f147a158b332cd8
Merge pull request #270 from djmitche/bug1472792

Bug 1472792 - allow multiple actions with matching name, choose by order

Comment 17

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

OK, I think this needs to get implemented in treeherder, then we can land the mozilla-central patch.  Hopefully that's a fairly small patch in TH.  Over to you, Hassan!

Comment 18

[Hassan Ali (:hassan)]

Thanks for your work Dustin. I will try to send a PR sometime this week.

Comment 19

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3778

Comment 20

[Hassan Ali (:hassan)]

This is now implemented in Treeherder via https://github.com/mozilla/treeherder/commit/4438b788f0c208e97743e470da7cb6e811c2a0b3.

Comment 21

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

Awesome, thanks!

Comment 22

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

I think this is deployed now, based on
  https://whatsdeployed.io/?owner=mozilla&repo=treeherder&name[]=Stage&url[]=https://treeherder.allizom.org/revision.txt&name[]=Prod&url[]=https://treeherder.mozilla.org/revision.txt

Comment 23

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

Whoops, I hadn't landed attachment 8990450 [details].  Landing it now with bug 1398277.

Comment 24

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c20ba876af1c686aa454d8e8e216828df04216a
Bug 1472792: give both retrigger actions the same name; r=bstack

Comment 25

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4c20ba876af1

Comment 26

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/4412639dcb7e

Comment 27

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/d2adfc87c259