1414919 - mach try fuzzy doesn't support --rebuild command

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I would like to be able to compare talos results and use mach try fuzzy for scheduling.

Comment 1

[Andrew Halberstadt [:ahal]]

This should be fairly simple to fix as long as we can assume that every task chosen by the fzf interface is going to be rebuilt. If we want to say "only rebuild these X tasks", then it will start getting complicated.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

there is --rebuild-talos, possibly we could create a warning to indicate that any value >10 should be used very sparingly.  Likewise, if they are --rebuild on >20 jobs, it should be confirmed with a secondary prompt.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

The motivation for this change is to create a 'rebuild' template that sets
'attributes.task_duplicates'. A nice side effect is that if any of 'task'
or 'attributes' aren't explicitly set, we can ignore the result which means
we don't need to have a whole bunch of 'else' clauses in the templates.

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

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1414919 - [taskgraph] Create a 'rebuild' morph template,

This template takes a single integer as input, and sets the 'task_duplicates'
attribute on any tasks that were explicitly specified by try_task_config.json.
This means dependent tasks or 'always_target' tasks will not be rebuilt.

To support this template, the apply_jsone_templates morph now requires the
entire try_task_config object instead of just the templates.

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

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1414919 - [tryselect] Add --rebuild support to |mach try fuzzy|,

This allows rebuilding all selected tasks. This defines an upper limit of
20 rebuilds per push. If more are needed, developers can either change it
in code or push multiple times.

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

Comment 6

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925976 [details]
Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

https://reviewboard.mozilla.org/r/197194/#review202314

::: commit-message-dc45e:3
(Diff revision 1)
> +The motivation for this change is to create a 'rebuild' template that sets
> +'attributes.task_duplicates'. A nice side effect is that if any of 'task'
> +or 'attributes' aren't explicitly set, we can ignore the result which means
> +we don't need to have a whole bunch of 'else' clauses in the templates.

This change breaks compatibility from the 'action task' format, which might mean it's a non-starter.

But so far there isn't anything shared between the two and the only alternative I can see would be to get the `target_tasks_method` to also read from the `templates` dict, which also seems less than ideal. The shared format might be a case of YAGNI.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8925978 [details]
Bug 1414919 - [tryselect] Add --rebuild support to |mach try fuzzy|,

https://reviewboard.mozilla.org/r/197198/#review202414

::: tools/tryselect/selectors/fuzzy.py:111
(Diff revision 1)
>           {'default': None,
>            'help': "Use the given parameters.yml to generate tasks, "
>                    "defaults to latest parameters.yml from mozilla-central",
>            }],
>      ]
> -    templates = ['artifact', 'env']
> +    templates = ['artifact', 'env', 'rebuild']

can we dynamically pick this up based on templates.py::all_templates.keys() ?

Comment 8

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

Comment on attachment 8925976 [details]
Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

https://reviewboard.mozilla.org/r/197194/#review202916

::: commit-message-dc45e:3
(Diff revision 1)
> +The motivation for this change is to create a 'rebuild' template that sets
> +'attributes.task_duplicates'. A nice side effect is that if any of 'task'
> +or 'attributes' aren't explicitly set, we can ignore the result which means
> +we don't need to have a whole bunch of 'else' clauses in the templates.

I think this is OK, but please update [the docs](https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/try.html#modifying-tasks-in-a-try-push) too.

Comment 9

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

Comment on attachment 8925977 [details]
Bug 1414919 - [taskgraph] Create a 'rebuild' morph template,

https://reviewboard.mozilla.org/r/197196/#review202918

Comment 10

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

Comment on attachment 8925976 [details]
Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

https://reviewboard.mozilla.org/r/197194/#review202920

BTW, the rationale for having the actions and morphs match was that we are likely to want to provide the same options either in a try push or in an action on a push after it was made.  If that's not borne out, then the match isn't so important.

Comment 11

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925978 [details]
Bug 1414919 - [tryselect] Add --rebuild support to |mach try fuzzy|,

https://reviewboard.mozilla.org/r/197198/#review202414

> can we dynamically pick this up based on templates.py::all_templates.keys() ?

That would work but I'd prefer to be explicit. If someone adds a new template in the future they might accidentally enable it for |mach try fuzzy| without meaning too. The fact that |mach try fuzzy| implements all of the currently existing templates is mostly coincidence.

Comment 12

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925976 [details]
Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

https://reviewboard.mozilla.org/r/197194/#review202920

Yeah, afaik there is no on working on that nor are there plans to. I agree it's nice to have that compatibility, but not worth holding up useful features over. If we decide compatibility becomes more important later on, we can make the necessary changes to bring them back in sync then.

Comment 13

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925976 [details]
Bug 1414919 - [taskgraph] Allow morph templates to also modify task attributes,

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

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925977 [details]
Bug 1414919 - [taskgraph] Create a 'rebuild' morph template,

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

Comment 15

[Andrew Halberstadt [:ahal]]

Comment on attachment 8925978 [details]
Bug 1414919 - [tryselect] Add --rebuild support to |mach try fuzzy|,

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b38e8ccf1219
[taskgraph] Allow morph templates to also modify task attributes, r=dustin
https://hg.mozilla.org/integration/autoland/rev/6d5a445dba8a
[taskgraph] Create a 'rebuild' morph template, r=dustin
https://hg.mozilla.org/integration/autoland/rev/7474dc45cbe3
[tryselect] Add --rebuild support to |mach try fuzzy|, r=jmaher

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b38e8ccf1219
https://hg.mozilla.org/mozilla-central/rev/6d5a445dba8a
https://hg.mozilla.org/mozilla-central/rev/7474dc45cbe3