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/13 09:09:28 UTC
[GitHub] [airflow] jhtimmins opened a new pull request #12332: Handle outdated session timeout gracefully.
jhtimmins opened a new pull request #12332:
URL: https://github.com/apache/airflow/pull/12332
This just updates the behavior when the webserver is started with outdated configuration values. Deprecated values are handled gracefully, rather than by shutting down the server.
----------------------------------------------------------------
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] kaxil commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523050793
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
+ logging.warning(
+ '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
+ 'renamed to `SESSION_LIFETIME_MINUTES`. The new option allows to configure '
+ 'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed '
Review comment:
```suggestion
'session lifetime in minutes. `force_logout_after` option has been removed '
```
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#issuecomment-727186596
The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523049721
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
+ logging.warning(
+ '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
+ 'renamed to `SESSION_LIFETIME_MINUTES`. The new option allows to configure '
+ 'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed '
+ 'from `webserver` section. Please update your configuration.'
Review comment:
```suggestion
'from `webserver` section. Please update your configuration.',
DeprecationWarning
```
----------------------------------------------------------------
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] ashb merged pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #12332:
URL: https://github.com/apache/airflow/pull/12332
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#issuecomment-727093109
[The Workflow run](https://github.com/apache/airflow/actions/runs/362525808) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523051483
##########
File path: airflow/www/app.py
##########
@@ -71,23 +69,7 @@ def create_app(config=None, testing=False, app_name="Airflow"):
flask_app = Flask(__name__)
flask_app.secret_key = conf.get('webserver', 'SECRET_KEY')
- if conf.has_option('webserver', 'SESSION_LIFETIME_DAYS') or conf.has_option(
- 'webserver', 'FORCE_LOG_OUT_AFTER'
- ):
- logging.error(
- '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
- 'renamed to `SESSION_LIFETIME_MINUTES`. New option allows to configure '
- 'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed '
- 'from `webserver` section. Please update your configuration.'
- )
- # Stop gunicorn server https://github.com/benoitc/gunicorn/blob/20.0.4/gunicorn/arbiter.py#L526
- sys.exit(4)
- else:
- session_lifetime_minutes = conf.getint('webserver', 'SESSION_LIFETIME_MINUTES', fallback=43200)
- logging.info('User session lifetime is set to %s minutes.', session_lifetime_minutes)
-
- flask_app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(minutes=session_lifetime_minutes)
-
+ flask_app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(minutes=conf.get_session_lifetime_config())
Review comment:
I wonder if we should make this a function in airflow.settings, rather than a method on config -- so far methods on config are (I think) all just generalized.
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523059379
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
Review comment:
This will fix your test failures:
```
airflow.exceptions.AirflowConfigException: section/key [webserver/session_lifetime_days] not found in config
```
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523200228
##########
File path: airflow/settings.py
##########
@@ -337,6 +337,31 @@ def prepare_syspath():
sys.path.append(PLUGINS_FOLDER)
+def get_session_lifetime_config():
+ """Gets session timeout configs and handles outdated configs gracefully."""
+ session_lifetime_days = conf.getint('webserver', 'session_lifetime_days', fallback=None)
+ if session_lifetime_days or conf.getint('webserver', 'force_logout_after', fallback=None):
+ warnings.warn(
+ '`session_lifetime_days` option from `[webserver]` section has been '
+ 'renamed to `session_lifetime_minutes`. The new option allows to configure '
+ 'session lifetime in minutes. The `force_logout_after` option has been removed '
+ 'from `[webserver]` section. Please update your configuration.',
+ category=DeprecationWarning,
+ )
Review comment:
We should only warn if session_lifetime_minutes is not also defined.
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523050013
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
Review comment:
```suggestion
session_lifetime_days = self.getint('webserver', 'session_lifetime_days', fallback=None)
if session_lifetime_days or self.getint('webserver', 'force_logout_after', fallback=None):
```
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
+ logging.warning(
+ '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
+ 'renamed to `SESSION_LIFETIME_MINUTES`. The new option allows to configure '
+ 'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed '
+ 'from `webserver` section. Please update your configuration.'
Review comment:
```suggestion
'from `webserver` section. Please update your configuration.',
DeprecationWarning,
```
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523412431
##########
File path: airflow/settings.py
##########
@@ -337,6 +337,33 @@ def prepare_syspath():
sys.path.append(PLUGINS_FOLDER)
+def get_session_lifetime_config():
+ """Gets session timeout configs and handles outdated configs gracefully."""
+ session_lifetime_minutes = conf.get('webserver', 'session_lifetime_minutes', fallback=None)
+ session_lifetime_days = conf.get('webserver', 'session_lifetime_days', fallback=None)
+ uses_deprecated_lifetime_configs = session_lifetime_days or conf.get(
+ 'webserver', 'force_logout_after', fallback=None
+ )
+
+ if uses_deprecated_lifetime_configs and not session_lifetime_minutes:
Review comment:
Hmmm, this is never going to trigger -- the default_airflow.cfg has a value of `session_lifetime_minutes`, which means that `conf.get('webserver', 'session_lifetime_minutes')` is always going to return a value (`43200`). HMMMM.
Perhaps we need a
```suggestion
if uses_deprecated_lifetime_configs and session_lifetime_minutes == '43200':
```
(I don't like it, but I can't see another option)
----------------------------------------------------------------
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] kaxil commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523050455
##########
File path: airflow/configuration.py
##########
@@ -705,6 +705,28 @@ def _warn_deprecate(section, key, deprecated_section, deprecated_name):
stacklevel=3,
)
+ def get_session_lifetime_config(self):
+ session_lifetime_days = self.getint('webserver', 'SESSION_LIFETIME_DAYS')
+ if session_lifetime_days or self.getint('webserver', 'FORCE_LOG_OUT_AFTER'):
+ logging.warning(
+ '`SESSION_LIFETIME_DAYS` option from `webserver` section has been '
Review comment:
nit
```suggestion
'`SESSION_LIFETIME_DAYS` option from `[webserver]` section has been '
```
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r523200301
##########
File path: airflow/settings.py
##########
@@ -337,6 +337,31 @@ def prepare_syspath():
sys.path.append(PLUGINS_FOLDER)
+def get_session_lifetime_config():
+ """Gets session timeout configs and handles outdated configs gracefully."""
+ session_lifetime_days = conf.getint('webserver', 'session_lifetime_days', fallback=None)
+ if session_lifetime_days or conf.getint('webserver', 'force_logout_after', fallback=None):
+ warnings.warn(
+ '`session_lifetime_days` option from `[webserver]` section has been '
+ 'renamed to `session_lifetime_minutes`. The new option allows to configure '
+ 'session lifetime in minutes. The `force_logout_after` option has been removed '
+ 'from `[webserver]` section. Please update your configuration.',
+ category=DeprecationWarning,
+ )
Review comment:
(Sorry missed this in earlier review)
----------------------------------------------------------------
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] ashb commented on a change in pull request #12332: Handle outdated session timeout gracefully.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #12332:
URL: https://github.com/apache/airflow/pull/12332#discussion_r524088119
##########
File path: airflow/settings.py
##########
@@ -337,6 +337,33 @@ def prepare_syspath():
sys.path.append(PLUGINS_FOLDER)
+def get_session_lifetime_config():
+ """Gets session timeout configs and handles outdated configs gracefully."""
+ session_lifetime_minutes = conf.get('webserver', 'session_lifetime_minutes', fallback=None)
+ session_lifetime_days = conf.get('webserver', 'session_lifetime_days', fallback=None)
+ uses_deprecated_lifetime_configs = session_lifetime_days or conf.get(
+ 'webserver', 'force_logout_after', fallback=None
+ )
+
+ if uses_deprecated_lifetime_configs and not session_lifetime_minutes:
Review comment:
We could do `session_lifetime_minutes == conf.airflow_defaults.get('webserver', 'session_lifetime_minutes')`
----------------------------------------------------------------
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