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/10/25 12:47:45 UTC
[GitHub] [airflow] potiuk opened a new pull request, #27260: Print important information outside of the folded output in CI
potiuk opened a new pull request, #27260:
URL: https://github.com/apache/airflow/pull/27260
This PR improves the ways important information ise printed in CI.
Currently some of the errors and important information is printed inside the folded console output when they are run. This is nice to have the folded content in case of success but in case of error we want to surface errors directly in the CI output so that the user does not have to find and unfold the information. Sometimes (for example in printing which dependencies got upgraded) we also want to surface some important part of the output even if there is a success.
This is a foundational change that makes it possible and easy, we are going to add more of those as we find them.
This change improves surfacing of the information in three cases:
1) Test failures
When there is an error in tests, we should now see the (only the part containing error and traceback) see the failure details outside of the folded content - the folded content will still contain all the details of the test run, but you would not have to unfold it in order to see the actual error.
2) Errors when testing provider installation
When there are errors when installing providers, the errors were initially inside of the folded "run" command and the run command did not even have indication that there was an error (just exit code information was printed). This change ends CI group when errors are detected (in CI) which makes the errors show up outside of the group.
3) Showing changed constraints
Showing changed constraints when upgrading to newer dependencies is always interesting - regardless from being it error or success. With this change, we are showing just the constraints printed in the main logs.
<!--
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] jedcunningham commented on a diff in pull request #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #27260:
URL: https://github.com/apache/airflow/pull/27260#discussion_r1005075046
##########
dev/breeze/src/airflow_breeze/utils/parallel.py:
##########
@@ -352,6 +354,11 @@ def check_async_run_results(
:param include_success_outputs: include outputs of successful parallel runs
:param poll_time: what's the poll time between checks
:param skip_cleanup: whether to skip cleanup of temporary files.
+ :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+ outputs should be printed as summary, so that you do not have to look at the folded details of
+ the run in CI
+ :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
Review Comment:
I'm good either way honestly, that's why I gave it the ✅. Probably just my familiarity bias: "of course it prints it on failure" 🙃.
--
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 a diff in pull request #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27260:
URL: https://github.com/apache/airflow/pull/27260#discussion_r1005071736
##########
dev/breeze/src/airflow_breeze/utils/parallel.py:
##########
@@ -352,6 +354,11 @@ def check_async_run_results(
:param include_success_outputs: include outputs of successful parallel runs
:param poll_time: what's the poll time between checks
:param skip_cleanup: whether to skip cleanup of temporary files.
+ :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+ outputs should be printed as summary, so that you do not have to look at the folded details of
+ the run in CI
+ :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
Review Comment:
Actually It will also print summary after failure.
The 'print_summary_after_success" name might be misleading (Yes i had it like that initially and when I used it, i realised that it can be misleading so I added `also`). I could change it to `also_print_summary_after_success` or smth like that but this is rather small thing :)
--
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 #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27260:
URL: https://github.com/apache/airflow/pull/27260#issuecomment-1291324891
Failure unrelated
--
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 a diff in pull request #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27260:
URL: https://github.com/apache/airflow/pull/27260#discussion_r1005071736
##########
dev/breeze/src/airflow_breeze/utils/parallel.py:
##########
@@ -352,6 +354,11 @@ def check_async_run_results(
:param include_success_outputs: include outputs of successful parallel runs
:param poll_time: what's the poll time between checks
:param skip_cleanup: whether to skip cleanup of temporary files.
+ :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+ outputs should be printed as summary, so that you do not have to look at the folded details of
+ the run in CI
+ :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
Review Comment:
Actually It will also print summary after failure.
The 'print_summary_after_succes" name might be misleading (Yes i had it like that initially and when I used it, i realised that it can be misleading so I added `also`). I could change it to `also_print_summary_after_success` or smth like that but this is rather small thing :)
--
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] jedcunningham commented on a diff in pull request #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #27260:
URL: https://github.com/apache/airflow/pull/27260#discussion_r1005028693
##########
dev/breeze/src/airflow_breeze/utils/parallel.py:
##########
@@ -352,6 +354,11 @@ def check_async_run_results(
:param include_success_outputs: include outputs of successful parallel runs
:param poll_time: what's the poll time between checks
:param skip_cleanup: whether to skip cleanup of temporary files.
+ :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+ outputs should be printed as summary, so that you do not have to look at the folded details of
+ the run in CI
+ :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
Review Comment:
```suggestion
:param print_summary_after_success: if summary is printed, this flag determines if summaries should
```
I think we can drop "also"?
--
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 a diff in pull request #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27260:
URL: https://github.com/apache/airflow/pull/27260#discussion_r1005082524
##########
dev/breeze/src/airflow_breeze/utils/parallel.py:
##########
@@ -352,6 +354,11 @@ def check_async_run_results(
:param include_success_outputs: include outputs of successful parallel runs
:param poll_time: what's the poll time between checks
:param skip_cleanup: whether to skip cleanup of temporary files.
+ :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+ outputs should be printed as summary, so that you do not have to look at the folded details of
+ the run in CI
+ :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
Review Comment:
Actually I changed that part to be more explicit:
```
summarize_on_ci=SummarizeAfter.SUCCESS,
summary_start_regexp=".*Constraints generated in.*",
```
and
```
summarize_on_ci=SummarizeAfter.FAILURE,
summary_start_regexp=r".*= FAILURES.*|.*= ERRORS.*",
```
Is much more explicit. And this way we have a bit more flexibility (SUCCESS/FAILURE/BOTH).
--
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 #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27260:
URL: https://github.com/apache/airflow/pull/27260#issuecomment-1290629288
This one should make it WAY easier to understand some of the CI errors because users will not have to open the "folded" parts of the output to see the actual errors (I will add more of those later as I encounter them).
Examples of this working:
1) Showing whch dependencies got upgraded:
https://github.com/apache/airflow/actions/runs/3321065827/jobs/5488222375
2) Errors while testing providers installation
https://github.com/apache/airflow/actions/runs/3321065827/jobs/5488433764
3) Failing unit tests:
https://github.com/apache/airflow/actions/runs/3321065827/jobs/5488434670
All of them surface the important error or output directly in the CI log without the information being folded and without the extra verbose information that is printed in the folded sections. Very similar to what we already do in docs building.
--
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 #27260: Print important information outside of the folded output in CI
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #27260:
URL: https://github.com/apache/airflow/pull/27260
--
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