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/09/04 10:44:19 UTC

[GitHub] [airflow] potiuk opened a new pull request, #26151: Update CI documentation, renaming runs to "Canary"

potiuk opened a new pull request, #26151:
URL: https://github.com/apache/airflow/pull/26151

   For quite some time we did not have the name of the main builds
   However more and more the "main" builds are used to provide early
   warnigns for some problems:
   
   * 3rd-party dependencies breaking our builds
   * our own dependencies breaking the constraints
   * building ARM images
   * building breeze images quickly
   * running complete matrix of tests
   * finding flaky tests
   
   So effectively, those main builds are really "Canary" builds - when
   those builds are failing, they give us a chance to react quickly,
   without affecting the regular PR builds.
   
   This PR clarifies the meaning and reasoning for those builds
   and introduces "Canary" name for them.
   
   During related documentation review, it also turned out that a number
   of old environment variables are not used any more (after the breeze
   changing to Python) and this PR also removes them from documentation
   and removes the variables from all the scripts (including removal of
   some unused scripts)
   
   The new documentation also mentions somethign that we've learned
   recently - that in case you use Breeze in non-airflow workflows in
   GitHub Actions, you need to override the variables through command
   line parameters rather than through environment variables, because
   GitHub actions treats GITHUB_* variables as immutable for security.
   
   <!--
   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 an 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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 #26151: Update CI documentation, renaming runs to "Canary"

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

   This is a bit of cleanup in the documentation of CI - introducing a "canary" name for our "main" workflow (which is basically what it is) and removing a number of variables that became unused by the recent changes of Breeze from bash to Python. 
   
   cc: @mik-laj -> since we had recently discussions about that, this change also makes the docs a bit clearer and give more guidance for anyone who wants to use breeze in their own workflow, making sure that they do not use GITHUB_* variables to override the variables coming from GA (as you cannot change them) 
   
   And also @o-nikolas -> since you wanted to understand more about the CI. I think this one describes more "whys" - changing the "main" build to be named "canary" and describing that PR runs are running in more "stable" environment. I think when you realise that this is the main purpose for having this comples CI with multiple run types makes it clearer why we have such setup. 
   
   There will be few more cleanups after we complete the last step of Breeze conversion. We will be able to remove few more variables,  and remove some remnants in the docs, but even now CI.rst is becoming more of a "WHY" description rather than "WHAT" description - it gets shorter and hopefully gives a chance to a passer-by to understand why we have all of the complexity.
   
   But if there are any suggestions on how we can improve those docs and describe it better, nicer, more concisely, maybe split it into separate documents, I am all ears.


-- 
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 #26151: Update CI documentation, renaming runs to "Canary"

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

   :D Any comments/approvals most welcome.


-- 
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 #26151: Update CI documentation, renaming runs to "Canary"

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

   (And PRs are most welcome of course),


-- 
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 #26151: Update CI documentation, renaming runs to "Canary"

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

   @ashb - this one also makes it quite a bit clearer why we are having the "merge" build at all and what purpose it serves - thanks to those renames the conditions in the CI.yml become quite a bit more "readable"


-- 
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 #26151: Update CI documentation, renaming runs to "Canary"

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26151:
URL: https://github.com/apache/airflow/pull/26151


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