Repository navigation
[packages] Override labels no longer update on re-runs #140232
Description
Activity
- addedpackageflutter/packages repository. See also p: labels.flutter/packages repository. See also p: labels.team-infraOwned by Infrastructure teamOwned by Infrastructure team
on Dec 15, 2023 - addedP1High-priority issues at the top of the work listHigh-priority issues at the top of the work listtriaged-infraTriaged by Infrastructure teamTriaged by Infrastructure team
on Dec 21, 2023 @stuartmorgan Do you have an example of the PR where the rerun didn't pick up the override labels? I don't think there is any changes related to that logic. As I see the current logic, if it's a rerun from the GitHub UI, the scheduler will pick up all the properties from the earlier failed build (which has the override label info) and inject it to the new build.
Do you have an example of the PR where the rerun didn't pick up the override labels?
I don't have a live one at the moment, but it should be trivial to create:
- Upload a PR with a change to code in a package, but not changing the version or CHANGELOG
- See the expected failure in
repo_checks - Add the labels
- Try re-running
if it's a rerun from the GitHub UI, the scheduler will pick up all the properties from the earlier failed build (which has the override label info)
I think you're describing the opposite of the failure case? Picking up exactly the state of the earlier failed build is the problem, because that earlier failed build doesn't have the labels.
The desired behavior, which we had originally, was that after the check fails we can simply add the labels to override it and then re-run check, which means it needs to read the live label state.
I am wondering if the logic ever works. The only case it would work (I can think of) is: if a pr is created, a label is added, a new commit is pushed, and then rerun on a failed task from the new commit.
Currently:
- when a new commit is pushed, the scheduler checks the labels and promote as build properties if the
overridelabel exists and then schedules the build. - when a rerun happens (though a new label is added before rerun), the schedule picks whatever properties the earlier failed build has and promotes them to the new to-be-scheduled build. Note here there is no further label check from the current PR.
What we should do (based on the desired behavior mentioned above):
- move the pr label checking logic to retry instead of the original commit level schedule, as we only want the label for retry case.
- when a new commit is pushed, the scheduler checks the labels and promote as build properties if the
I'm sure I tested that it worked without pushing a new commit when this was first set up, because that was so important to the desired workflow (and because it had at some point broken in Cirrus for a while, and that was miserable until we fixed it).
Hum, then I am not sure. But flutter/cocoon#3403 should fix the issue.
- added a commit that references this issue
on Jan 9, 2024 Great, thanks!
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.- locked as resolved and limited conversation to collaborators
on Jan 24, 2024
#130076 added support for the
flutter/packagesworkflow of adding labels that allow us to override specific checks. The intended behavior, and the way it worked when first deployed, is that if we added an override label to a PR and then re-ran the failing task, it would pass.At some point since then the behavior has changed, and re-running is no longer picking up the current label state. Instead, the only way to get the checks to pass after adding the labels is to push a new commit to the PR. This is a major regression in functionality; the task that runs the check these labels override is explicitly designed to be fast, so instead of a workflow of "add label, re-run a fast task, PR is green a minute or two later" it's "add a label, do something that pushes a commit, which restarts all tests, potentially making it take 30+ minutes to get a landable PR".
/cc @yusuf-goog