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 2020/10/19 00:53:48 UTC

[GitHub] [airflow] kaxil opened a new pull request #11648: Revert 3

kaxil opened a new pull request #11648:
URL: https://github.com/apache/airflow/pull/11648


   The static checks are failing, even after the 2 PRs that were merged to fix the issue. For now reverting them is the best option
   
   Example: #11645 (which is rebased on latest master and contains 4fcc71c)
   
   <!--
   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/master/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/master/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.

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



[GitHub] [airflow] kaxil merged pull request #11648: Revert "Optimizes CI builds heavily with selective checks (#11541)"

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11648:
URL: https://github.com/apache/airflow/pull/11648


   


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



[GitHub] [airflow] potiuk commented on pull request #11648: Revert "Optimizes CI builds heavily with selective checks (#11541)"

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11648:
URL: https://github.com/apache/airflow/pull/11648#issuecomment-711773515


   Good call! I tried to reverse engineer what GitHub is doing for PRs (it's not documented at all) but it is somewhat inconsistent:
   
   - in the workflow_run - where we retrieve merge_conmmit via API, it's always OK to build the image using this commit (this is what we do for quite a while and it works fine) 
   
   However in the pull_request workflow the behavior is somewhat complex)  and depends on whether this is a push, pull_request from your own repo, pull_request from a forked repo, how many commits are in in the PR and whether they are based on the latest master or some earlier version :exploding_head: 
   
   - sometimes there is a merge commit containing all the changes.
   - sometimes there is an empty merge commit that contains no changes (!) only when you run difference vs. parent you see the list of files
   - sometimes the merge commit is null
   
   And sometimes those commits are not available by default because the fetch strategy by GitHub Action is to only fetch the latest commit. 
   
   I tried to only use that single commit to make it works but I think it is dangerous to reverse-engineer this. so I will revert to the original strategy - I will find out the Head of incoming commit and compare it with the head of the target branch. This worked quite fine for a while but it involves a bit more steps.
   
   


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