jedcunningham commented on a change in pull request #21167:
URL: https://github.com/apache/airflow/pull/21167#discussion_r799874678



##########
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 think we should:
   - hard code the table name (otherwise we'd need to use it in the migration 
too, and honestly I don't think it's worth it)
   - remove the `SESSION_KEY_PREFIX` (we wont ever share a backend)
   - force `SESSION_USE_SIGNER` to True (we already require a secret_key)




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


Reply via email to