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/04 15:50:51 UTC
[GitHub] [airflow] SamWheating opened a new pull request #12811: Collapse the importErrors pane by default
SamWheating opened a new pull request #12811:
URL: https://github.com/apache/airflow/pull/12811
<!--
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:
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Our Airflow Environment is shared by multiple teams and many users. On several occasions we've seen someone introduce an error in a couple hundred generated DAGs, which completely clobbers the web UI.
My initial plan was to introduce a separate view for import errors, and allow users to disable the import errors on the main page via configuration. However, now that Airflow 2 makes this section of the page collapsible, I figured its easier to just make this section collapsed by default. This prevents one user's errors from affecting other users' experience.
Before:
After:
Happy to discuss, if you disagree with this change in default behaviour then feel free to close.
---
**^ 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] ryanahamilton commented on a change in pull request #12811: Collapse the importErrors pane by default
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #12811:
URL: https://github.com/apache/airflow/pull/12811#discussion_r536365656
##########
File path: airflow/www/templates/appbuilder/flash.html
##########
@@ -53,13 +53,13 @@
<div class="panel panel-default">
<div class="panel-heading" id="errorHeading">
<h4 class="panel-title">
- <a id="alerts-accordion-toggle" data-toggle="collapse" data-parent="#accordion" href="#alerts" aria-expanded="false">
+ <a id="alerts-accordion-toggle" data-toggle="collapse" data-parent="#accordion" href="#alerts" aria-expanded="false" class="collapsed">
DAG Import Errors ({{ dag_import_errors|length }})
Review comment:
Since the red error message blocks won't be visible by default anymore, I think it would be good to add some emphasis to the to toggle with color and an icon—something like this:
```suggestion
<span class="text-danger"><span class="material-icons" aria-hidden="true">error</span> DAG Import Errors ({{ dag_import_errors|length }})</span>
```
----------------------------------------------------------------
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] ryanahamilton merged pull request #12811: Collapse the importErrors pane by default
Posted by GitBox <gi...@apache.org>.
ryanahamilton merged pull request #12811:
URL: https://github.com/apache/airflow/pull/12811
----------------------------------------------------------------
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] ryanahamilton commented on pull request #12811: Collapse the importErrors pane by default
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #12811:
URL: https://github.com/apache/airflow/pull/12811#issuecomment-740716476
Test failure appears unrelated to the is PR.
----------------------------------------------------------------
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] SamWheating commented on a change in pull request #12811: Collapse the importErrors pane by default
Posted by GitBox <gi...@apache.org>.
SamWheating commented on a change in pull request #12811:
URL: https://github.com/apache/airflow/pull/12811#discussion_r536382706
##########
File path: airflow/www/templates/appbuilder/flash.html
##########
@@ -53,13 +53,13 @@
<div class="panel panel-default">
<div class="panel-heading" id="errorHeading">
<h4 class="panel-title">
- <a id="alerts-accordion-toggle" data-toggle="collapse" data-parent="#accordion" href="#alerts" aria-expanded="false">
+ <a id="alerts-accordion-toggle" data-toggle="collapse" data-parent="#accordion" href="#alerts" aria-expanded="false" class="collapsed">
DAG Import Errors ({{ dag_import_errors|length }})
Review comment:
Yeah this looks a lot better, thanks for the suggestion!
----------------------------------------------------------------
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