blag commented on a change in pull request #21167:
URL: https://github.com/apache/airflow/pull/21167#discussion_r798837969
##########
File path: airflow/www/extensions/init_session.py
##########
@@ -15,33 +15,30 @@
# specific language governing permissions and limitations
# under the License.
-from flask import request, session as flask_session
-from flask.sessions import SecureCookieSessionInterface
+from flask import session as builtin_flask_session
+from airflow.configuration import conf
+from airflow.www.session import AirflowDatabaseSessionInterface,
AirflowSecureCookieSessionInterface
-class AirflowSessionInterface(SecureCookieSessionInterface):
- """
- Airflow cookie session interface.
- Modifications of sessions should be done here because
- the change here is global.
- """
- def save_session(self, *args, **kwargs):
- """Prevent creating session from REST API requests."""
- if request.blueprint == '/api/v1':
- return None
- return super().save_session(*args, **kwargs)
-
-
-def init_permanent_session(app):
- """Make session permanent to allows us to store data"""
-
- def make_session_permanent():
- flask_session.permanent = True
-
- app.before_request(make_session_permanent)
-
-
-def init_airflow_session_interface(app):
+def init_airflow_session_interface(app, db):
"""Set airflow session interface"""
- app.session_interface = AirflowSessionInterface()
+ config = app.config.copy()
+ selected_backend = conf.get('webserver', 'SESSION_BACKEND',
fallback='database')
+
+ if selected_backend == 'securecookie':
+ app.session_interface = AirflowSecureCookieSessionInterface()
+ if config.get('SESSION_PERMANENT', True):
+
+ def make_session_permanent():
+ builtin_flask_session.permanent = True
+
+ app.before_request(make_session_permanent)
+ elif selected_backend == 'database' or selected_backend is None:
+ app.session_interface = AirflowDatabaseSessionInterface(
+ db,
+ config.get('SESSION_SQLALCHEMY_TABLE', 'sessions'),
+ config.get('SESSION_KEY_PREFIX', 'session:'),
+ config.get('SESSION_USE_SIGNER', True),
+ config.get('SESSION_PERMANENT', True),
+ )
Review comment:
I'm not sure how well this will fly, but I'd like to leave these
undocumented - for now.
The upstream Flask-Session package supported these options, presumably
because some users needed them. But every option we add is more branches and
more code to possibly break and debug, so I'd like to keep the number of
configurable options to a minimum.
So for these, I'd like to keep these available as escape hatches **for the
interim** as we switch (most? all?) users over to database-backed sessions. If
none of our users need these options then we can remove them and simply hard
code the values. If we do have users who need these options, then we can
explicitly document (and support) them later.
But if we really want to surface these options to users or if we want to
hard code them out of the gate, I'm happy to make that change.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
