You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/26 14:02:40 UTC
[GitHub] [airflow] potiuk opened a new pull request #22542: Optimize direct push workflows in GitHub Actions
potiuk opened a new pull request #22542:
URL: https://github.com/apache/airflow/pull/22542
When the build is run via direct push to apache airflow repo
we do not need to run two separate workflows. The "push" workflow
is never a "pull request from fork" so it should have
the capability to build and push images to registry.
This allows the committers to make direct push requests to run PRs
that are actually running the build without having to merge
build-image.yml first.
This is cool because committers can simply push a branch to apache
and test if it works with some build image changes that otherwise
would require to push to `main` of an apache-airflow fork.
Another advantage is that merge and schedule builds do not run two
separate workflows - both building the image and running tests is done
in the same workflow (and the build-image workflow is not started)
This saves some build time on "wait for CI images" and
"wait for PROD images" jobs - because in merge builds and
schedule builds they only start after the images are actually
built.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079702548
Hey everyone (especially @ashb - you wanted to implement it differently but I found a better way) - this is really nice optimization of the build workflow of ours for "direct pushes", "merges", "scheduled builds" and "PRs not from forks".
Those do not happen for regular PRs but it will allow to test "build changing" PRs by committers and it will save quite some time for "wait for images" for merge and scheduled builds.
This has been inspired by the changes I had to implement in `v2-2-tests` branch to make sure that the builds will run fine for "Python 3.6" (which is not supported in `main` any more) - it turned out that I could simply add conditional "build images" jobs in the `ci.yml` workflow. All steps for those will be "skipped" in "fork" PRs (because they have no permissions) and in this case the "build-images" workflow is still used but for all builds that are done in the "apache/airlfow" repo, it's perfectly fine to build images in "ci.yml" workflow.
Also cc: @edithturn - this will make it much easier to get in changes that also change build scripts - because for all the PRs and pushes done directly in `apache/airflow` (only committers can push there) we will not have to merge to `main` in order to verify that builds changing the build scripts work. Just internal PR from "apache/airlfow" will be enough
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079773533
> @potiuk, how could this be optimized without changing Selective Check, which is what pretty much defines the entire CI flow? Pd. They are really beautiful diagrams ✈️
Because selective checks are one level below. The changes are about the workflow setup - and selective checks are just one step in the workflows/diagrams:
```
Note over Tests: Build info<br>Decide on tests<br>Decide on Matrix (selective)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079757095
> Haven't given it an in depth look.
I pushed a few more changes and fixes. I also revised and vastly simplified the diagrams showing what's going on:
https://github.com/apache/airflow/blob/5ba65d0d78b564a64a9effaf1448b162bb17a5f1/CI_DIAGRAMS.md
This might be now easy to review. I will also run more tests for all scenarios before merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079705450
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079757095
> Haven't given it an in depth look.
I pushed a few more changes and fixes. I also revised and vastly simplified the diagrams showing what's going on:
https://github.com/apache/airflow/blob/278714fa32e56f393ded8130111b6e89c59b0220/CI_DIAGRAMS.md
This might be now easy to review. I will also run more tests for all scenarios before merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] edithturn commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
edithturn commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079771593
@potiuk, how could this be optimized without changing Selective Check, which is what pretty much defines the entire CI flow?
Pd. They are really beautiful diagrams ✈️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079765462
cc: @jedcunningham - you wrote recently you want to learn more about the CI workflow :). https://github.com/apache/airflow/pull/22542#:~:text=https%3A//github.com/apache/airflow/blob/278714fa32e56f393ded8130111b6e89c59b0220/CI_DIAGRAMS.md is a good start :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22542:
URL: https://github.com/apache/airflow/pull/22542#issuecomment-1079777129
Seems it works for the PR cases. https://github.com/apache/airflow/pull/22545 and https://github.com/apache/airflow/pull/22542 look good. Merging to finally test mere PR an schedule one tonight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #22542: Optimize direct push workflows in GitHub Actions
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22542:
URL: https://github.com/apache/airflow/pull/22542
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org