[GitHub] [airflow] potiuk commented on a diff in pull request #25056: Disable provider packages building for non-main branch builds

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org