Closed Bug 1696995 Opened 9 months ago Closed 8 months ago

Drop single match requirement in `evaluate_keyed_by`

Categories

(Firefox Build System :: Task Configuration, task, P2)

task

Tracking

(firefox89 fixed)

RESOLVED FIXED
89 Branch
Tracking Status
firefox89 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file)

In the evaluate_keyed_by function (used to resolve the by-<key> regexes, we error out if a label would be matched by more than one regex.

In my opinion, this restriction is unnecessary and leads to complicated regexes. For example, I had to write something like this to satisfy that requirement:

run-on-projects:
     by-test-platform:
        .*(-ccov|-asan|-shippable).*: []
        .*(linux|windows)(?!.*(-ccov|-asan|-shippable)).*-qr.*: []
        default: ['integration']

The second regex needed to duplicate the -ccov|-asan|-shippable part using a negative lookahead since otherwise a label like test-linux-shippable-qr/... would get matched by both regexes and the taskgraph would fail.

Instead I propose that these regexes operate more like a case statement, where the value of the first matching regex gets applied. If we wanted to play it safe, we could introduce a strict parameter which enables the single regex behaviour on a case by case basis (so we could opt out of this behaviour in particular locations if we wanted.

Or we could just drop the condition wholesale and all internalize the new case statement-like behaviour. I vote on this one, but if anyone has valid reasons to keep the status quo I can be persuaded to make it opt-in.

It's worth noting that this proposal hinges on our ability to preserve key ordering.

Dict keys are ordered in 3.6 (as an implementation detail), but in Python 3.7 they are ordered as part of the spec
so we'd need to upgrade to 3.6 (or preferably 3.7). Alternatively we could use a yaml loader that uses OrderedDict on older Pythons. Looks like oyaml at least has this feature, not sure it's worth switching over though (I'd rather just require a newer Python).

Depends on: 1696944

At least in the 'test' kind, this is causing the keyed_by dicts to get very
complex as we need to ensure that each task is only matched by a single regex.
Instead, this makes them act more like case / match statements where the first
arm that matches the task will be used.

Assignee: nobody → ahal
Status: NEW → ASSIGNED
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1cf213ff5e8c
[taskgraph] Drop requirement single match requirement in 'resolve_keyed_by' for test kind, r=taskgraph-reviewers,aki
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
You need to log in before you can comment on or make changes to this bug.