Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only run builds on "push" events for v* test branch #31825

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 9, 2023

We do not need to run separate build on push and pull request when we are cherry-picking and pushing branch to apache/airflow repo. Those PRs are always made by committers, and they are done as direct push to the apache/airflow repo so they will run build on push (which will be equivalent of canary run). We also do not need to run anything on stable, because stable is the result of merging those changes from test after they get green.


^ 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.rst or {issue_number}.significant.rst, in newsfragments.

We do not need to run separate build on push and pull request
when we are cherry-picking and pushing branch to apache/airflow
repo. Those PRs are always made by committers, and they are done
as direct push to the apache/airflow repo so they will run build
on push (which will be equivalent of canary run). We also do not
need to run anything on stable, because stable is the result of
merging those changes from test after they get green.
@potiuk
Copy link
Member Author

potiuk commented Jun 9, 2023

This one will avoid running two builds (one for push and one for pull_request) when we are working on *test branch.

@potiuk
Copy link
Member Author

potiuk commented Jun 9, 2023

Another finding from 2.6.2 preparation

@potiuk potiuk requested a review from jedcunningham June 9, 2023 22:11
@potiuk
Copy link
Member Author

potiuk commented Jun 9, 2023

This likely led to "permission error" when two builds at teh same time tried to push the same image in v2-6-test branch

@jedcunningham
Copy link
Member

We also do not need to run anything on stable, because stable is the result of merging those changes from test after they get green.

Doesn't the run on stable update constraint files though?

@potiuk
Copy link
Member Author

potiuk commented Jun 9, 2023

We also do not need to run anything on stable, because stable is the result of merging those changes from test after they get green.

Doesn't the run on stable update constraint files though?

Glad you asked :)

Nope. The "v*-test" one updates constraints immediately if the test build succeeds. We do not need to defer updating constraints - this is only needed for PR runs (the reason why we do not update constraints for PRs is that they will break other's PRs if pushed too early).

This is controlled by "canary-run" flag, which is set by "get-workflow-info":

    def is_canary_run(self) -> str:
        if (
            self.event_name == "push"
            and self.head_repo == "apache/airflow"
            and self.ref_name
            and (self.ref_name == "main" or TEST_BRANCH_MATCHER.match(self.ref_name))
        ):
            return "true"
        return "false"

@potiuk potiuk merged commit 0dddaa0 into apache:main Jun 9, 2023
@potiuk potiuk deleted the only-run-build-for-pushes-in-non-main-branch branch June 9, 2023 23:04
potiuk added a commit that referenced this pull request Jun 9, 2023
We do not need to run separate build on push and pull request
when we are cherry-picking and pushing branch to apache/airflow
repo. Those PRs are always made by committers, and they are done
as direct push to the apache/airflow repo so they will run build
on push (which will be equivalent of canary run). We also do not
need to run anything on stable, because stable is the result of
merging those changes from test after they get green.

(cherry picked from commit 0dddaa0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants