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 2022/02/09 21:01:42 UTC
[GitHub] [airflow] jedcunningham opened a new pull request #21478: Alt: Add a session backend to store session data in the database
jedcunningham opened a new pull request #21478:
URL: https://github.com/apache/airflow/pull/21478
This is an alternate approach to #21167, the primary difference being we don't need to vendor in Flask-Session.
The downside, however, is the weirdness of having our own session model that isn't actually used other than to remove the table during `db reset`. Is there a better way to accomplish that?
--
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] ashb commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r806129671
##########
File path: setup.cfg
##########
@@ -96,6 +96,7 @@ install_requires =
flask-appbuilder~=3.4, <4.0.0
flask-caching>=1.5.0, <2.0.0
flask-login>=0.3, <0.5
+ flask-session>=0.3.2
Review comment:
Yeah, I'd say pin this - don't forget a comment saying why we have done 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803090543
##########
File path: setup.cfg
##########
@@ -96,6 +96,7 @@ install_requires =
flask-appbuilder~=3.4, <4.0.0
flask-caching>=1.5.0, <2.0.0
flask-login>=0.3, <0.5
+ flask-session>=0.3.2
Review comment:
As it is now, I'm tempted to pin this to 0.4.0, as we'd need a migration for any schema changes in the model anyways.
--
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] ephraimbuddy commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803472895
##########
File path: airflow/migrations/versions/c381b21cb7e4_add_session_table_to_db.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add session table to db
+
+Revision ID: c381b21cb7e4
+Revises: be2bfac3da23
+Create Date: 2022-01-25 13:56:35.069429
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'c381b21cb7e4'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+TABLE_NAME = 'session'
+
+
+def upgrade():
+ """Apply add session table to db"""
+ op.create_table(
Review comment:
I think we should also add an ORM model for this table so that it's not only through migration that the db can be created.
I'm working on a change that would have new DBs created through the ORM rather than going through the migration files. https://github.com/apache/airflow/pull/21462
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803283186
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
No longer an issue 🎉
--
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] ashb commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r806129671
##########
File path: setup.cfg
##########
@@ -96,6 +96,7 @@ install_requires =
flask-appbuilder~=3.4, <4.0.0
flask-caching>=1.5.0, <2.0.0
flask-login>=0.3, <0.5
+ flask-session>=0.3.2
Review comment:
Yeah, I'd say pin this - don't forget a comment saying why we have done 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803088979
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
it seems to me that a simple database table definition, if that is all that is common, is not meaningful enough to warrant attribution
--
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] ephraimbuddy commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803472895
##########
File path: airflow/migrations/versions/c381b21cb7e4_add_session_table_to_db.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add session table to db
+
+Revision ID: c381b21cb7e4
+Revises: be2bfac3da23
+Create Date: 2022-01-25 13:56:35.069429
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'c381b21cb7e4'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+TABLE_NAME = 'session'
+
+
+def upgrade():
+ """Apply add session table to db"""
+ op.create_table(
Review comment:
I think we should also add an ORM model for this table so that it's not only through migration file that the table can be created.
I'm working on a change that would have new DBs created through the ORM rather than going through the migration files. https://github.com/apache/airflow/pull/21462
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803283091
##########
File path: airflow/utils/db.py
##########
@@ -55,6 +55,9 @@
# We need to add this model manually to get reset working well
from airflow.models.serialized_dag import SerializedDagModel # noqa: F401
+
+# Alias this to WebSession so it doesn't conflict with sqla.orm.session.Session
+from airflow.models.session import Session as WebSession # noqa: F401
Review comment:
No longer an issue 🎉
--
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] jedcunningham merged pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #21478:
URL: https://github.com/apache/airflow/pull/21478
--
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 #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#issuecomment-1034230671
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803085519
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
I'm not sure if the model itself is enough to warrant keeping the copyright notice and addition to NOTICE. Thoughts?
--
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] ashb commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803536348
##########
File path: airflow/migrations/versions/c381b21cb7e4_add_session_table_to_db.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add session table to db
+
+Revision ID: c381b21cb7e4
+Revises: be2bfac3da23
+Create Date: 2022-01-25 13:56:35.069429
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'c381b21cb7e4'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+TABLE_NAME = 'session'
+
+
+def upgrade():
+ """Apply add session table to db"""
+ op.create_table(
Review comment:
(If we should or not is the next question)
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r806143627
##########
File path: Dockerfile.ci
##########
@@ -26,7 +26,7 @@ ARG PYTHON_BASE_IMAGE
ARG AIRFLOW_IMAGE_REPOSITORY="https://github.com/apache/airflow"
# By increasing this number we can do force build of all dependencies
-ARG DEPENDENCIES_EPOCH_NUMBER="6"
+ARG DEPENDENCIES_EPOCH_NUMBER="7"
Review comment:
I just realized this only needs to be bumped when _removing_ dependencies. Fix incoming.
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r806143627
##########
File path: Dockerfile.ci
##########
@@ -26,7 +26,7 @@ ARG PYTHON_BASE_IMAGE
ARG AIRFLOW_IMAGE_REPOSITORY="https://github.com/apache/airflow"
# By increasing this number we can do force build of all dependencies
-ARG DEPENDENCIES_EPOCH_NUMBER="6"
+ARG DEPENDENCIES_EPOCH_NUMBER="7"
Review comment:
I just realized this only needs to be bumped when _removing_ dependencies. Fix incoming.
##########
File path: setup.cfg
##########
@@ -96,6 +96,7 @@ install_requires =
flask-appbuilder~=3.4, <4.0.0
flask-caching>=1.5.0, <2.0.0
flask-login>=0.3, <0.5
+ flask-session>=0.3.2
Review comment:
Pretty much same difference, but I set the upper bound at the latest release instead of pinning to just the latest version.
(oh, and fixed the lower bound that I messed up originally 🙃)
--
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] blag commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
blag commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803221759
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
I would err on the side of caution and keep the attribution.
--
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] ashb commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803512496
##########
File path: airflow/migrations/versions/c381b21cb7e4_add_session_table_to_db.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add session table to db
+
+Revision ID: c381b21cb7e4
+Revises: be2bfac3da23
+Create Date: 2022-01-25 13:56:35.069429
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'c381b21cb7e4'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+TABLE_NAME = 'session'
+
+
+def upgrade():
+ """Apply add session table to db"""
+ op.create_table(
Review comment:
We can do it like this I think
```python
from flask.session import SqlAlchemySessionInterface
from flask_appbuilder import SQLA
...
db = SQLA()
db.session = settings.Session
SessionModel = SqlAlchemySessionInterface(db=db).sql_session_model
```
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r804214993
##########
File path: airflow/migrations/versions/c381b21cb7e4_add_session_table_to_db.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""add session table to db
+
+Revision ID: c381b21cb7e4
+Revises: be2bfac3da23
+Create Date: 2022-01-25 13:56:35.069429
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'c381b21cb7e4'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+TABLE_NAME = 'session'
+
+
+def upgrade():
+ """Apply add session table to db"""
+ op.create_table(
Review comment:
This is what it ends up looking like, when considering #21462's changes:
```
--- a/airflow/utils/db.py
+++ b/airflow/utils/db.py
@@ -606,12 +606,21 @@ def initdb(session: Session = NEW_SESSION):
from airflow.models.base import Base
from airflow.www.app import create_app
+ from airflow.www.session import AirflowDatabaseSessionInterface
+ from flask_sqlalchemy import SQLAlchemy
+
Base.metadata.create_all(settings.engine)
# Stamp migration head with alembic
config = _get_alembic_config()
command.stamp(config, "head")
# sets up the db; Sync permissions etc
- create_app(config={'UPDATE_FAB_PERMS': True})
+ app = create_app(config={'UPDATE_FAB_PERMS': True})
+
+ # Create `session` table
+ db = SQLAlchemy(app)
+ AirflowDatabaseSessionInterface(app=app, db=db, table='session', key_prefix='')
+ db.create_all()
+
if conf.getboolean('core', 'LOAD_DEFAULT_CONNECTIONS'):
create_default_connections(session=session)
```
--
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] ashb commented on pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#issuecomment-1034451255
https://github.com/apache/airflow/blob/ee20a03908a2f3ad37dbfcec71a19f37f1446088/airflow/utils/db.py#L1057
We can drop the table like this without needing a model
--
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] dstandish commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803088979
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
it seems to me that a simple database table definition, if that is all that is common, is not meaningful enough
--
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] dstandish commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803088227
##########
File path: airflow/models/session.py
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# ~~~~~~~~~~~~~~~~~~~~~~
+#
+# Server-side Sessions and SessionInterfaces.
+#
+# Originally copied from flask_session.sessions
+# Flask-Session
+# Copyright (C) 2014 by Shipeng Feng and other contributors
+# BSD, see LICENSE for more details.
+from datetime import datetime
+from typing import Any, Dict
+
+from sqlalchemy import Column, DateTime, Integer, LargeBinary, String
+
+from airflow.models.base import Base
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+# This simply matches the model in `fask_session.sessions.SqlAlchemySessionInterface`
+class Session(Base, LoggingMixin):
Review comment:
it's a good question, firmly outside my bailywick, but intuition says _no_
--
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] dstandish commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r803121386
##########
File path: airflow/www/session.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from flask import request
+from flask.sessions import SecureCookieSessionInterface
+from flask_session.sessions import SqlAlchemySessionInterface
+
+
+class SesssionExemptBlueprintMixin:
Review comment:
name makes it seem like it's a blueprint or specific to blueprints but it seems a little more general e.g. exempting the `health` endpoint
##########
File path: airflow/utils/db.py
##########
@@ -55,6 +55,9 @@
# We need to add this model manually to get reset working well
from airflow.models.serialized_dag import SerializedDagModel # noqa: F401
+
+# Alias this to WebSession so it doesn't conflict with sqla.orm.session.Session
+from airflow.models.session import Session as WebSession # noqa: F401
Review comment:
maybe we should name the model itself WebSession so as to not conflict?
##########
File path: airflow/www/extensions/init_session.py
##########
@@ -15,33 +15,44 @@
# 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
-
-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)
+from airflow.configuration import conf
+from airflow.exceptions import AirflowConfigException
+from airflow.www.session import AirflowDatabaseSessionInterface, AirflowSecureCookieSessionInterface
def init_airflow_session_interface(app):
"""Set airflow session interface"""
- app.session_interface = AirflowSessionInterface()
+ config = app.config.copy()
+ selected_backend = conf.get('webserver', 'SESSION_BACKEND')
+ # A bit of a misnomer - normally cookies expire whenever the browser is closed
+ # or when they hit their expiry datetime, whichever comes first. "Permanent"
+ # cookies only expire when they hit their expiry datetime, and can outlive
+ # the browser being closed.
+ permanent_cookie = config.get('SESSION_PERMANENT', True)
+
+ if selected_backend == 'securecookie':
+ app.session_interface = AirflowSecureCookieSessionInterface()
+ if permanent_cookie:
+
+ def make_session_permanent():
+ builtin_flask_session.permanent = True
+
+ app.before_request(make_session_permanent)
+ elif selected_backend == 'database':
+ app.session_interface = AirflowDatabaseSessionInterface(
+ app=app,
+ db=None,
+ # These options come from Flask-Session, but are hard-coded
Review comment:
meaning of this comment could be made a little clearer
--
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] jedcunningham commented on a change in pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21478:
URL: https://github.com/apache/airflow/pull/21478#discussion_r806154905
##########
File path: setup.cfg
##########
@@ -96,6 +96,7 @@ install_requires =
flask-appbuilder~=3.4, <4.0.0
flask-caching>=1.5.0, <2.0.0
flask-login>=0.3, <0.5
+ flask-session>=0.3.2
Review comment:
Pretty much same difference, but I set the upper bound at the latest release instead of pinning to just the latest version.
(oh, and fixed the lower bound that I messed up originally 🙃)
--
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] jedcunningham merged pull request #21478: Alt: Add a session backend to store session data in the database
Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #21478:
URL: https://github.com/apache/airflow/pull/21478
--
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