[GitHub] [airflow] mik-laj commented on a change in pull request #11745: Unify user session lifetime configuration

[GitBox <gi...@apache.org>]

mik-laj commented on a change in pull request #11745:
URL: https://github.com/apache/airflow/pull/11745#discussion_r510427359



##########
File path: airflow/www/app.py
##########
@@ -66,8 +66,16 @@ def create_app(config=None, testing=False, app_name="Airflow"):
     flask_app = Flask(__name__)
     flask_app.secret_key = conf.get('webserver', 'SECRET_KEY')
 
-    session_lifetime_days = conf.getint('webserver', 'SESSION_LIFETIME_DAYS', fallback=30)
-    flask_app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(days=session_lifetime_days)
+    if conf.has_option('webserver', 'SESSION_LIFETIME_DAYS'):

Review comment:
       I think we should not warn users about configuration options that do not work if the user has the correct configuration options for Airflow 2.0. This will allow us to use one configuration file for Airflow 1.10 and 2.0.  This is essential if users want to use Helm Chart and other generic deployment tools.
   
   So I propose that you check if there is a `SESSION_LIFETIME_MINUTES` option in the config file. If so, use it.
   If it is not there, we should display a deprecation warning and we should try to maintain backward compatibility.  In this case, this means that the warning should display a message that option `SESSION_LIFETIME_DAYS` has been renamed to `SESSION_LIFETIME_MINUTES` and the unit from day to hour and that option `FORCE_LOG_OUT_AFTER` has been removed.
   
   When this change has been merged, please create a ticket requesting a new rule for [airflow upgrade-check](https://github.com/apache/airflow/issues/8765).
   




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