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/12/26 18:40:54 UTC
[GitHub] [airflow] potiuk opened a new pull request #13323: Vastly improves usability of CI logs
potiuk opened a new pull request #13323:
URL: https://github.com/apache/airflow/pull/13323
This change introduces improvements in the way logs are displayed
in CI jobs and in amount of logs produced in general for CI jobs
due to much smarter cache usage.
Logs in all CI jobs are now grouped in groups which are folded
by default when there is no error generated in such group. Similar
solution has been already used in docs job and it improved
both readability and speed of loading of the logs in CI after
recent improvements in Github UI (previously the speed of loading
the logs was not improved by groups).
Also cache usage has been reviewed and fixed in a number of places
which will result in much shorted setup times for static checks
and kubernetes virtualenv but also far shorter logs generated by
cache setup (we are using restore-keys feature that implements
incremental approach for cache building even if cache keys in
GitHub Actions are immutable.
<!--
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] potiuk commented on pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#issuecomment-751384858
I have really high hopes for this one - it should make the logs of all CI jobs much better to read/use. They should load much faster and only show unfolded groups when there is an error. This should be especially visible in Tests (Each test type will get their own group - folded by default) and especially failed K8S tests.
Those failed tests in case of K8S generate vast amount of logs when there is a failure and it makes the interface nearly unusable - now each container should have it's own named and folded log section.
The recent change in GitHub Actions UI made it load really fast if most of the logs are in folded groups at which point this change started to make sense (@mik-laj implemented it for docs and it has proven to work really well after GA UI revamp).
----------------------------------------------------------------
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] XD-DENG commented on a change in pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#discussion_r549102378
##########
File path: .github/workflows/ci.yml
##########
@@ -393,12 +390,12 @@ jobs:
run: ./scripts/ci/provider_packages/ci_prepare_provider_packages.sh
- name: "Install and test provider packages and airflow via ${{ matrix.package-format }} files"
run: ./scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh
- - name: "Upload provider package artifacts"
+ - name: "Upload package artifacts"
uses: actions/upload-artifact@v2
if: always()
with:
name: airflow-backport-packages
- path: "./files/airflow-packages-*"
+ path: "./dist/apache*"
Review comment:
May you share the reason behind this change?
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -898,18 +912,14 @@ function build_images::determine_docker_cache_strategy() {
function build_images::build_prod_images_from_packages() {
+ start_end::group_start "PIP download"
Review comment:
This is duplicated with what you have as 1st line inside function `runs::run_pip_download`
##########
File path: scripts/ci/libraries/_start_end.sh
##########
@@ -16,6 +16,26 @@
# specific language governing permissions and limitations
# under the License.
+# Starts group for Github Actions - makes logs much more readable
+function start_end::group_start {
+ if [[ ${GITHUB_ACTIONS=} == "true" ]]; then
Review comment:
```suggestion
if [[ ${GITHUB_ACTIONS} == "true" ]]; then
```
Should it be like this instead? The same for the one below in `function start_end::group_end` as well?
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#issuecomment-751510446
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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 #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#issuecomment-751451908
Some screenshots from logs:
----------------------------------------------------------------
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 a change in pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#discussion_r549131129
##########
File path: .github/workflows/ci.yml
##########
@@ -393,12 +390,12 @@ jobs:
run: ./scripts/ci/provider_packages/ci_prepare_provider_packages.sh
- name: "Install and test provider packages and airflow via ${{ matrix.package-format }} files"
run: ./scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh
- - name: "Upload provider package artifacts"
+ - name: "Upload package artifacts"
uses: actions/upload-artifact@v2
if: always()
with:
name: airflow-backport-packages
- path: "./files/airflow-packages-*"
+ path: "./dist/apache*"
Review comment:
Yeah. It was left-over from previous version of the action. I fixed all the warnings I encountered while making the change.
----------------------------------------------------------------
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 a change in pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#discussion_r549131652
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -898,18 +912,14 @@ function build_images::determine_docker_cache_strategy() {
function build_images::build_prod_images_from_packages() {
+ start_end::group_start "PIP download"
Review comment:
Ah yeah. Good one !
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#issuecomment-751489513
[The Workflow run](https://github.com/apache/airflow/actions/runs/447374664) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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 a change in pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#discussion_r549131593
##########
File path: scripts/ci/libraries/_start_end.sh
##########
@@ -16,6 +16,26 @@
# specific language governing permissions and limitations
# under the License.
+# Starts group for Github Actions - makes logs much more readable
+function start_end::group_start {
+ if [[ ${GITHUB_ACTIONS=} == "true" ]]; then
Review comment:
Nope. It's OK. We have 'set -u` (fail on undefined) set in the script so it will fail if a variable is undefined. It helps to catch typos but in this case GITHUB_ACTIONS might be undefined when run locally so I (out of precaution) add = to make the variable value empty rather than undefined.
----------------------------------------------------------------
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 merged pull request #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13323:
URL: https://github.com/apache/airflow/pull/13323
----------------------------------------------------------------
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 #13323: Vastly improves usability of CI logs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13323:
URL: https://github.com/apache/airflow/pull/13323#issuecomment-751510568
Added last one small update for "selective_checks"
----------------------------------------------------------------
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