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/11/14 04:05:36 UTC

[GitHub] [airflow] Acehaidrey opened a new pull request #12365: Make import errors dropdown configurable for default value

Acehaidrey opened a new pull request #12365:
URL: https://github.com/apache/airflow/pull/12365


   <!--
   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/
   -->
   
   ---
   This is a simple pr to introduce the option to have the import errors collapsed. I know some teams / orgs like to see the import errors front and center (makes sense) but for such large organizations with thousands of flows, it could be the noise generated from import errors taking up a lot of real estate here. This pr is to introduce an optional config to make it collapsing by default.
   
   I pass in this config to the view, and use a bit of a statement to make this possible. I.e. to add the `in` property or not. If there are better ways to do this please chime in!
   **^ 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 pull request #12365: Make import errors dropdown configurable for default value

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


   I know it seems harmless, but I don't love the idea of using a config flag to determine this behavior. The accumulation of these types of "opt-in features" will contribute to a more fragmented overall product experience IMO.
   
   Just thinking out loud, maybe the `flask_session` could be utilized to store the last toggle state on a per-user basis?
   
   I do wonder if collapsing it by default could be an acceptable experience for everyone if we updated the presentation to stand out a bit more (like the mock below)? This would certainly be the easiest solution, but I'd be interested to get others' opinions on this.
   
   ![image](https://user-images.githubusercontent.com/3267/99272269-99753c00-27f5-11eb-8e15-694ff6603c05.png)
   


----------------------------------------------------------------
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 #12365: Make import errors dropdown configurable for default value

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


   I know it seems harmless, but I don't love the idea of using a config flag 
   
   I do wonder if collapsing it by default could be an acceptable experience for everyone if we updated the presentation to stand out a bit more (like the mock below)?
   
   ![image](https://user-images.githubusercontent.com/3267/99272269-99753c00-27f5-11eb-8e15-694ff6603c05.png)
   


----------------------------------------------------------------
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 removed a comment on pull request #12365: Make import errors dropdown configurable for default value

Posted by GitBox <gi...@apache.org>.
ryanahamilton removed a comment on pull request #12365:
URL: https://github.com/apache/airflow/pull/12365#issuecomment-728135884


   I know it seems harmless, but I don't love the idea of using a config flag 
   
   I do wonder if collapsing it by default could be an acceptable experience for everyone if we updated the presentation to stand out a bit more (like the mock below)?
   
   ![image](https://user-images.githubusercontent.com/3267/99272269-99753c00-27f5-11eb-8e15-694ff6603c05.png)
   


----------------------------------------------------------------
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 #12365: Make import errors dropdown configurable for default value

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


   Hey @Acehaidrey I'm going to go ahead and close this PR in favor of #12811 that delivers the same desired outcome. Thank you!


----------------------------------------------------------------
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 closed pull request #12365: Make import errors dropdown configurable for default value

Posted by GitBox <gi...@apache.org>.
ryanahamilton closed pull request #12365:
URL: https://github.com/apache/airflow/pull/12365


   


----------------------------------------------------------------
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] Acehaidrey commented on pull request #12365: Make import errors dropdown configurable for default value

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


   Hi @ryanahamilton thank youo much for letting me know and yes I prefer this solution as well :). Thank you team! 


----------------------------------------------------------------
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] Acehaidrey commented on pull request #12365: Make import errors dropdown configurable for default value

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


   Hey @ryanahamilton thank you for the feedback!
   
   I am completely onboard with that. To make the UI stand out a bit like you had it, and have it collapsed. 
   
   I only did it this way to support how things currently are, and what users may prefer - but prefer your mentionings personally. 


----------------------------------------------------------------
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 closed pull request #12365: Make import errors dropdown configurable for default value

Posted by GitBox <gi...@apache.org>.
ryanahamilton closed pull request #12365:
URL: https://github.com/apache/airflow/pull/12365


   


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