search

taskcluster / taskgraph

Generates task dependency graphs for Taskcluster CI
Mozilla Public License 2.0
16 stars 42 forks source link

fix!: only add target_tasks_method filter if no other filters were sp… #600

Closed ahal closed 1 month ago

ahal commented 1 month ago

…ecified

BREAKING CHANGE: The target_tasks_method is now only implicitly added if no other filters exist.

This is needed to fix a bug in Gecko's ./mach try infrastructure, however the change makes sense on its own. Currently there's no way to forego target_tasks_method, and this allows us to do that.

Furthermore, there are no other built-in filters, so it's unlikely that this will impact any existing consumers.

ahal commented 1 month ago

Yes it does make it possible to by-pass the target_tasks_method filters. I'll give a brief explanation of why I need this in Gecko here, but it might be easier to explain over zoom.

In bug 1925007 I discovered that it was basically impossible to select the set of tasks that a cron task selects with ./mach try. This was immediately painful for Joel and I as we are working on the os-integration tests. But obviously would be a nice ability to have in general as well.

The reason it's not possible to select the same set of tasks, is because we override the target_tasks_method in mach try here and here.

We do this because this because usually we don't want to use whatever the normal target_tasks_method is for the project. E.g, we don't want to do the same thing as we would on a normal autoland push, we want to schedule the tasks that are listed in try_task_config.json instead.

However, in some cases we do want to use the original target_tasks_method. Cron decision tasks are a great example. We want to select from the same set of tasks that the cron task selects, however because we've overwritten target_tasks_method, that information is lost and it is no longer possible.

There's probably other ways of solving this, but my solution is here: https://phabricator.services.mozilla.com/D226554

Essentially it converts the special "tryselect" target_tasks_methods into filters. Filters are pretty much identical to target_task_methods, and it looks like whoever added them was intending for it to replace target_tasks entirely. Tbh, up until today I was thinking we should just remove the concept of filters (because I don't really see the benefit in migrating at this point).. But this seems to be a valid use case of them?

If you have other ideas to solve the issue, I'm open to them!

ahal commented 1 month ago

We discussed further on zoom and Ben has no concerns with landing this.