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 2021/03/25 01:42:07 UTC

[GitHub] [airflow] potiuk commented on a change in pull request #14996: Skips provider package builds and provider tests for non-master

potiuk commented on a change in pull request #14996:
URL: https://github.com/apache/airflow/pull/14996#discussion_r600984082



##########
File path: .github/workflows/ci.yml
##########
@@ -504,12 +505,13 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
     strategy:
       matrix:
         package-format: ['wheel', 'sdist']
-    if: needs.build-info.outputs.image-build == 'true'
+    if: needs.build-info.outputs.image-build == 'true' && needs.build-info.outputs.default-branch == 'master'

Review comment:
       OK. You asked this question, be prepared for the answer :). It's long, sorry, but GitHub did NOT make it easy.
   
   Beware. It's long even if the question is simple.
   
   The problem is that if you are running direct push (i.e. merge to master) you have the actual master branch in `github.ref`, indeed, however it looks completely differently in other workflows.
   
   When you run `pull_request` workflow, github ref is something like `refs/pull/14996/merge`. In this case you should look for ``github.base_ref`` instead:
   ```
   github.base_ref	string	The base_ref or target branch of the pull request in a workflow run. This property is only available when the event that triggers a workflow run is a pull_request.
   ```
   And - of course - it is not documented. 
   
   And  ... wait for it ..  base_ref is NOT set in direct pushes at all (i.e. when you merge) so if you would like to use this information, your condition will start looking like:
   
   ```
   if github.event != 'pull_request' && github.base_ref == 'master' || github.ref == 'master'  
   ```
   
   But, unfortunately, this is just a beginning. I am not 100% sure if we are covered here because `github.ref` behaviour for`pull_request` workflows is not documented (many of the values in GitHub context are poorly documented or not documented at all and I had to reverse-engineer them). 
   
   What's more - in `pull_request_workflow'  you have no information whatsoever what was the base branch. Not even when you query the "source_run" directly via GitHub API. This is because (surprise-surprise) when you query the "triggering" workflow from the "triggered" one - you have no access to the "context" of the triggering run (obviously). You only can see whatever the API returns you and the last time when I checked there was no information about the "base_branch" there. It could have changed since though.
   
   So what you have to do (I did not find a better way) is that you have to find the original PR via GitHub API and retrieve information from there. But in order to retrieve the information from there you need to iterate through open PRs and match the PR by branch + repo (I am already doing it to be able to push "Status checks" to the PR. Yes. I know, it is insane. But I could not find another way.
   
   If you are adventurous and want to look yourself - in every job we have this step starting from "Event " ... where I dump the context - this makes reverse-engineering much easier (but none more pleasant).
   
   AFAIK there is no sane way to get the 'base" branch.
   
   That's why we have DEFAULT_BRANCH in _initialization.sh that we keep different in every branch and I am using it. And exposing it as "output" seems like much simpler thing to do.
   
   I would be super happy if you could find an easier way here.
   
   
   

##########
File path: .github/workflows/ci.yml
##########
@@ -504,12 +505,13 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
     strategy:
       matrix:
         package-format: ['wheel', 'sdist']
-    if: needs.build-info.outputs.image-build == 'true'
+    if: needs.build-info.outputs.image-build == 'true' && needs.build-info.outputs.default-branch == 'master'

Review comment:
       OK. You asked this question, be prepared for the answer :). It's long, sorry, but GitHub did NOT make it easy.
   
   Beware. It's long even if the question is simple.
   
   The problem is that if you are running direct push (i.e. merge to master) you have the actual master branch in `github.ref`, indeed, however it looks completely differently in other workflows.
   
   When you run `pull_request` workflow, github ref is something like `refs/pull/14996/merge`. In this case you should look for ``github.base_ref`` instead:
   ```
   github.base_ref	string	The base_ref or target branch of the pull request in a workflow run. This property is
    only available when the event that triggers a workflow run is a pull_request.
   ```
   And - of course - it is not documented. 
   
   And  ... wait for it ..  base_ref is NOT set in direct pushes at all (i.e. when you merge) so if you would like to use this information, your condition will start looking like:
   
   ```
   if github.event != 'pull_request' && github.base_ref == 'master' || github.ref == 'master'  
   ```
   
   But, unfortunately, this is just a beginning. I am not 100% sure if we are covered here because `github.ref` behaviour for`pull_request` workflows is not documented (many of the values in GitHub context are poorly documented or not documented at all and I had to reverse-engineer them). 
   
   What's more - in `pull_request_workflow'  you have no information whatsoever what was the base branch. Not even when you query the "source_run" directly via GitHub API. This is because (surprise-surprise) when you query the "triggering" workflow from the "triggered" one - you have no access to the "context" of the triggering run (obviously). You only can see whatever the API returns you and the last time when I checked there was no information about the "base_branch" there. It could have changed since though.
   
   So what you have to do (I did not find a better way) is that you have to find the original PR via GitHub API and retrieve information from there. But in order to retrieve the information from there you need to iterate through open PRs and match the PR by branch + repo (I am already doing it to be able to push "Status checks" to the PR. Yes. I know, it is insane. But I could not find another way.
   
   If you are adventurous and want to look yourself - in every job we have this step starting from "Event " ... where I dump the context - this makes reverse-engineering much easier (but none more pleasant).
   
   AFAIK there is no sane way to get the 'base" branch.
   
   That's why we have DEFAULT_BRANCH in _initialization.sh that we keep different in every branch and I am using it. And exposing it as "output" seems like much simpler thing to do.
   
   I would be super happy if you could find an easier way here.
   
   
   




-- 
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.

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