Skip to content

[packages] Override labels no longer update on re-runs #140232

Description

@stuartmorgan-g

#130076 added support for the flutter/packages workflow 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

Activity

  1. added
    packageflutter/packages repository. See also p: labels.
    team-infraOwned by Infrastructure team
    on Dec 15, 2023
  2. added
    P1High-priority issues at the top of the work list
    triaged-infraTriaged by Infrastructure team
    on Dec 21, 2023
  3. keyonghan commented on Jan 9, 2024

    @keyonghan
    Contributor

    @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.

  4. stuartmorgan-g commented on Jan 9, 2024

    @stuartmorgan-g
    ContributorAuthor

    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.

  5. keyonghan commented on Jan 9, 2024

    @keyonghan
    Contributor

    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 override label 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.
  6. stuartmorgan-g commented on Jan 9, 2024

    @stuartmorgan-g
    ContributorAuthor

    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).

  7. self-assigned this
    on Jan 9, 2024
  8. keyonghan commented on Jan 9, 2024

    @keyonghan
    Contributor

    Hum, then I am not sure. But flutter/cocoon#3403 should fix the issue.

  9. stuartmorgan-g commented on Jan 10, 2024

    @stuartmorgan-g
    ContributorAuthor

    Great, thanks!

  10. github-actions commented on Jan 24, 2024

    @github-actions

    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 -v and a minimal reproduction of the issue.

  11. locked as resolved and limited conversation to collaborators on Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P1High-priority issues at the top of the work listpackageflutter/packages repository. See also p: labels.team-infraOwned by Infrastructure teamtriaged-infraTriaged by Infrastructure team

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions