potiuk commented on code in PR #25056:
URL: https://github.com/apache/airflow/pull/25056#discussion_r924361243
##########
.github/workflows/build-images.yml:
##########
@@ -319,12 +319,10 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
key:
"pre-commit-${{steps.host-python-version.outputs.host-python-version}}-\
${{ hashFiles('.pre-commit-config.yaml') }}"
restore-keys:
pre-commit-${{steps.host-python-version.outputs.host-python-version}}
- if: needs.build-info.outputs.default-branch == 'main'
Review Comment:
Not really.
More context:
The "default-branch" is the "target" of your Pull Request. I.e. if you are
making a PR using v2-3-stable or v2-3-test, your "default-branch" will be
v2-3-test. When you base your PR on main, it will be 'main'. This is controlled
by different value in
https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/branch_defaults.py
in those two branches.
This is a bit cumbersome, I know and seems that we could do it differently,
but unfortunately we cannot do it differently :(. Or I could not find a good
way at least.
I thought a lot about it in the past and the only way to figure out which
"major.minor" of Airflow the change refers to, we need to commit the branch
version to the code (and this is what "branch_defaults.py" is all about).
For most (but not all) PRs, we could probably figure out (more-or-less)
whether this is a "main" change or "2.3" or "2.2" change from the PR - we could
see the "target branch" and assume that if it is "main" - > this would be
"main" branch, if it is `v2-3-stable` or `v2-3-test` -> this would be 2.3. But
this stops working as long as we allow direct push builds and people doing
internal PRs in their forks. In those cases, there is no relation between the
branch name or target branch you have and the "line" of airlfow the branch
comes from. There is simply not enough metadata information in git fork to be
able to determine that with certaintly.
So the easiest way to determine "which" line of airflow the change comes
from is to have the "2.3" or "main" embedded in the source code in the
repository. This is precisely what "branch_defaultts.py" does. This is
deliberately a small, separate file so that we do not accidentally override it
when we cherry-pick changes. Simply the only change that happens to that file
is at the moment you create a new branch (which reminds me that we should
update it in
https://github.com/apache/airflow/blob/main/dev/README_RELEASE_AIRFLOW.md#update-default-branches)
:) after we changed to Python breeze.
This file does not change at all through the lifetime of the branch - so it
will never get overwritten during cherry-picks.
Also - when we retrieve the "default branch" in "build-images.yml" - we
cannot "execute" this file - we need to get the file from the incoming change,
and we cannot execute any code coming from PR outside of the docker build
(becaaw os security and potential bitcoing mining with our self-hosted
runners). This is the reason why in build-image.yml we simply READ the content
of that file and find the default branch from it :).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
