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 2021/09/30 19:17:28 UTC

[GitHub] [airflow] turbaszek opened a new pull request #18652: Add banner indicating environment type

turbaszek opened a new pull request #18652:
URL: https://github.com/apache/airflow/pull/18652


   This work was inspired by responses to this [linkedin comment](https://www.linkedin.com/feed/update/urn:li:activity:6848963597859618816?commentUrn=urn%3Ali%3Acomment%3A%28activity%3A6848963597859618816%2C6848966678429138944%29). The aim is to add simple banner hovering at top of the Airflow webui and indicating what environment type it is (test, staging, production etc).
   
   Example:
   <img width="1680" alt="Screenshot 2021-09-30 at 21 13 05" src="https://user-images.githubusercontent.com/9528307/135516488-cd9a2249-c8ea-4e3a-93e7-1f93ec955659.png">
   
   
   The banner is by default disabled. Personally I think we should provided users with predefined environment types with correlated colours (to keep the UI visually consistent). However, I'm extremely poor front developer so if someone would like to help with polishing the css and html I would be grateful ❤️ 
   
   ![](https://media.giphy.com/media/13FrpeVH09Zrb2/giphy.gif?cid=ecf05e47rtbejrchvsara8ohngjlm54av315fg6z5z8tyzoa&rid=giphy.gif&ct=g)


-- 
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 edited a comment on pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18652:
URL: https://github.com/apache/airflow/pull/18652#issuecomment-932974022


   I agree with @turbaszek  that it is a good idea. Actually the fact that you need to explain to the users something with "Did you actually know that you can do this and this to achieve that"? means that there is something wrong with the feature - it's either not obvious, not discoverable, or maybe it has many overlapping uses, which make it difficult to understand and even realise that you can already do something (albeit in a convoluted way).
   
   There are various reasons why you would like to change instance name, or change navbar colour - for example due to matching your colors with the company "color theme" or naming your instances via department. However explicitly naming the option "as environment"  which would be "orthognal" to that and much more visible is a good idea:
   
   * it serves the explicit use case and only that one (without blocking the other use cases i mentioned above).
   
   * I think this use case is rather popular and very much "technical" one - in terms that it is good to have something "extra" shown in "dev/staging" environments, to clearly distinguish them and let the user know that this environment is not the one which is "production". Using other elements (already existing ones but modifying them) is not as "visually distinctive" and prevents from making efficient matrixes of cases (say introducing STAGING/DEV/PROD environment for all 10 DEPARTMENTS which have different names while matching the navbar with the company colours). 
   
   * the use case is often  requesteded and popular and we have a chance the user to find answer on "how I should configure staging environment" in the docs and configuration of Airflow rather than in Stack Overflow / LinkedIn posts. 
   
   
   I would be very much for explicit "environment" flag like @turbaszek is proposing.
   


-- 
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 #18652: Add banner indicating environment type

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


   I agree with @turbaszek  that it is a good idea. Actually the fact that you need to explain to the users something with "Did you actually know that you can do this and this to achieve that"? means that there is something wrong with the feature - it's either not obvious, not discoverable, or maybe it has many overlapping uses, which make it difficult to understand and even realise that you can already do something (albeit in a convoluted way).
   
   There are various reasons why you would like to change instance name, or change navbar colour - for example due to matching your colors with the company "color theme" or naming your instances via department. However explicitly naming the option "as environment"  which would be "orthognal" to that and much more visible is a good idea:
   
   * it serves the explicit use case and only that one (without blocking the other use cases i mentioned above).
   
   * I think this use case is rather popular and very much "technical" one - in terms that it is good to have something "extra" shown in "dev/staging" environments, to clearly distinguish them and let the user know that this environment is not the one which is "production". Using other elements (already existing ones but modifying them) is not as "visually distinctive".
   
   * the use case is often  requesteded and popular and we have a chance the user to find answer on "how I should configure staging environment" in the docs and configuration of Airflow rather than in Stack Overflow / LinkedIn posts. 
   
   
   I would be very much for explicit "environment" flag like @turbaszek is proposing.
   


-- 
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] turbaszek commented on a change in pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #18652:
URL: https://github.com/apache/airflow/pull/18652#discussion_r719691519



##########
File path: airflow/www/templates/appbuilder/flash.html
##########
@@ -21,14 +21,13 @@
   Adapted from: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/templates/appbuilder/flash.html
 -#}
 <link rel="stylesheet" type="text/css" href="{{ url_for_asset('flash.css') }}">
-
+<div style="padding-top: 40px">

Review comment:
       Did that because otherwise the error was slightly hidden by navbar




-- 
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] turbaszek commented on pull request #18652: Add banner indicating environment type

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


   > How is this feature different from `[webserver] instance_name`? I have the impression that the purpose is very similar.
   > 
   > > ## instance_name
   > > New in version 2.1.0.
   > > Sets a custom page title for the DAGs overview page and site title for all pages
   > 
   > http://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#id29
   
   I agree that both can be used to achieve the same thing. The difference is visibility. If you will take a look at the thread I linked you will see that changing navbar colour is quite common practice to achieve more visible distinction than just changing a word (instance name). I think we can combine those two options, WDYT? 


-- 
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] bbovenzi commented on pull request #18652: Add banner indicating environment type

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


   UX-wise I think it would be better to integrate the env into the existing navbar-header instead of tacking on another banner.
   
   Also, I'm happy to help with CSS :)


-- 
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 change in pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18652:
URL: https://github.com/apache/airflow/pull/18652#discussion_r720844803



##########
File path: airflow/config_templates/config.yml
##########
@@ -1177,6 +1177,20 @@
       type: string
       example: ~
       default: "#fff"
+    - name: enable_environment_banner
+      description: |
+        Enable banner indicating environment type of the instance
+      version_added: ~
+      type: boolean
+      example: ~
+      default: "False"
+    - name: environment_type
+      description: |
+        Environment type of this Airflow instance. One of PROD, STAGING, TEST.

Review comment:
       ```suggestion
           Environment type of this Airflow instance. Example would be PROD, STAGING, TEST.
   ```




-- 
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] github-actions[bot] closed pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #18652:
URL: https://github.com/apache/airflow/pull/18652


   


-- 
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] github-actions[bot] commented on pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18652:
URL: https://github.com/apache/airflow/pull/18652#issuecomment-973498562


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
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] mik-laj commented on pull request #18652: Add banner indicating environment type

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18652:
URL: https://github.com/apache/airflow/pull/18652#issuecomment-931616473


   How is this feature different from `[webserver] instance_name`?  I have the impression that the purpose is very similar.
   
   > ## instance_name
   > New in version 2.1.0.
   > 
   > Sets a custom page title for the DAGs overview page and site title for all pages
   
   http://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#id29


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