Fix re-running failed jobs by committers without rebase #31792
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
potiuk
force-pushed
the
fix-rerunning-failed-jobs-by-committers
branch
from
7b1e2a0 to
a8ec922
Compare
potiuk
force-pushed
the
fix-rerunning-failed-jobs-by-committers
branch
from
a8ec922 to
80c02ce
Compare
Member
Author
|
cc: @o-nikolas and others - that should fix the situation when you are "just" rerunning someone's job without rebasing and it fails immediately. The #31451 changed the decision to run on to use "actor" which was incompatible with runner check. This caused - for example - attempts to re-run the jobs fail in #30637 |
Member
Author
|
This also caused attempts to re-run fail in https://github.com/apache/airflow/pull/30367/checks @uranusjr |
vincbeck
approved these changes
Jun 8, 2023
jedcunningham
approved these changes
Jun 8, 2023
Member
There was a problem hiding this comment.
Suggested change
| f"[warning]The {actor} retrieved as actor from GITHUB_CONTEXT's" | |
| f"[info]{actor} retrieved as actor from GITHUB_CONTEXT's" |
2 things:
- Don't need "the" here, even if "The potiuk retrieved as actor" does sounds good :)
- Shouldn't this be an info, not warning?
Member
Author
There was a problem hiding this comment.
Warning is better. The colors to show are specifically designed to be easily distinguishable.
- Don't need "the" here, even if "The potiuk retrieved as actor" does sounds good :)
I wanted to start the sentence capitalized.
Member
Author
There was a problem hiding this comment.
I changed it to:
f"[warning]The actor: {actor} retrieved from GITHUB_CONTEXT's"
After apache#31451 there were cases where re-running failed builds by committers without rebasing the PR could have caused that the runs were scheduled on self-hosted runners, but the runner rejected the build because it came from non-trusted actor. This PR brings back the calculation of runs-on based on PR user login rather than on the actor of the build.
potiuk
force-pushed
the
fix-rerunning-failed-jobs-by-committers
branch
from
80c02ce to
8508dc8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After #31451 there were cases where re-running failed builds by committers without rebasing the PR could have caused that the runs were scheduled on self-hosted runners, but the runner rejected the build because it came from non-trusted actor.
This PR brings back the calculation of runs-on based on PR user login rather than on the actor of the build.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.