Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix init & multiple app issues with SQLAlchemy interface #12

Closed
wants to merge 2 commits into from

Conversation

rsyring
Copy link

@rsyring rsyring commented Jun 11, 2015

These are some proposed fixes to issues I have run into with integrating Flask-Session into our application. The two problems were:

  1. the call to db.create_all() was causing problems as described here: SQLAlchemy db.create_all() causing errors #11
  2. When instantiating an application multiple times in the same process (in our case, during testing) the method used to create the SA table/model would fail b/c it already existed. But a check in for this so it's only created once for each DB.

rsyring and others added 2 commits June 11, 2015 08:49
Extensions are typically supposed to work for multiple instances of an app
in a single process.  This adjusts the creation of the Session model so
there is only one model regardless of how many apps are instantiated.
@nZac
Copy link

nZac commented Apr 26, 2016

@fengsp it would be very helpful to have this fixed. What can I do to get it moving?

@fengsp
Copy link
Contributor

fengsp commented Apr 28, 2016

@rsyring We need to call the create_all. Otherwise, how to do that for users? The session table detail is hidden. Any good ideas?

return '<Session data %s>' % self.data

self.sql_session_model = db.session_ext_session_model = Session
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we don't need the else?

@nZac
Copy link

nZac commented Apr 28, 2016

@fengsp my suggestion would be to create an entities.py file, and create a SessionMixin class which the user of library could include in their own SQLA metadata / entities however they want.

You might also provide an alembic script in the docs with a raw sql version if developers are trying to migrate.

Flask-Session would then, as part of the init_app be handed the entity which should be used to construct a session object.

@revmischa
Copy link

The create_all call is not necessary, we deleted it in our branch because it was problematic.

@kwiersma
Copy link

kwiersma commented Mar 1, 2017

Instead of the create_all() call you could use Session.__table__.create(bind=db.engine, checkfirst=True). That will create the table for the session model if it doesn't already exist.

@revmischa
Copy link

moving to flask-session 0.3.1 from 0.3.0 made the flask_session table stop getting created when doing my initial alembic schema migration. something changed that broke our use case. how are we supposed to deploy the flask_session table now when doing an initial schema migration/deploy?

@fengsp
Copy link
Contributor

fengsp commented Apr 6, 2017

@revmischa You should do the db creation by yourself, including your other models.

@tassaron
Copy link

tassaron commented Jan 1, 2021

The fix for recreating the session model is still very needed. I have to fork this repo in order to do tests, or else I get errors from the test suite as it tries to redefine this table on every initialization of the plugin

@behai-nguyen
Copy link

I am using 0.4.0, and am still having this problem:

-- I have around 396 pytest cases. For 3 ( three ) particular test cases, if I run only them together, two ( 2 ) would fail.

I came to this post,

https://stackoverflow.com/questions/64721498/how-do-you-resolve-already-defined-in-this-metadata-instance-error-with-flask
How do you resolve 'Already defined in this MetaData Instance' Error with Flask Pytest, SqlAlchemy, and Flask Sessions?

I saw @tassaron answer. If I modified flask_session/sessions.py directly, the way @tassaron does:

class SqlAlchemySessionInterface(SessionInterface):
...
    def __init__(self, app, db, table, key_prefix, use_signer=False,
                 permanent=True):
...
        if table not in self.db.metadata:
            class Session(self.db.Model):
...
            # self.db.create_all()
            self.sql_session_model = db.session_ext_session_model = Session
        else:
            self.sql_session_model = db.session_ext_session_model

My problems go away. We do need:

        else:
            self.sql_session_model = db.session_ext_session_model

Otherwise later on it would complain self.sql_session_model not exist.

Thank you and best regards.

...behai.

@behai-nguyen
Copy link

This is how I get around this problem. It feels like a hack though:

"""
Flask-Session version 0.4.0 -- latest as on 24/November/2022.

Work around / hack for getting around the exception during tests:

sqlalchemy.exc.InvalidRequestError: Table 'sessions' is already defined 
for this MetaData instance. Specify 'extend_existing=True' to redefine 
options and columns on an existing Table object.

Usage:

try:
    from book_keeping.library.fixed_session import FixedSession as Session
except ImportError:
    from flask_session import Session


References:

https://stackoverflow.com/questions/64721498/how-do-you-resolve-already-defined-in-this-metadata-instance-error-with-flask
https://github.com/fengsp/flask-session/blob/1c1f7903184673682bd1d75432c8f455b62393a4/flask_session/sessions.py
https://github.com/tassaron/muffin-shop/blob/main/src/helpers/main/session_interface.py
https://github.com/fengsp/flask-session/pull/12
"""
from flask_session.sessions import SqlAlchemySessionInterface
from flask_session import Session

class FixedSqlAlchemySessionInterface( SqlAlchemySessionInterface ):
    def __init__(self, app, db, table, key_prefix, use_signer=False,
                 permanent=True):
        """
        Assumption: the way I use it, db is always a valid instance 
        at this point.
        """
        if table not in db.metadata:
            super().__init__( app, db, table, key_prefix, use_signer, permanent )
            db.session_ext_session_model = self.sql_session_model
        else:
            # print( "`sessions` table already exists..." )

            self.db = db
            self.key_prefix = key_prefix
            self.use_signer = use_signer
            self.permanent = permanent
            self.has_same_site_capability = hasattr(self, "get_cookie_samesite")

            self.sql_session_model = db.session_ext_session_model
            
class FixedSession( Session ):
    def _get_interface(self, app):
        config = app.config.copy()

        if config[ 'SESSION_TYPE' ] != 'sqlalchemy':
            return super()._get_interface( app )

        else:
            config.setdefault( 'SESSION_PERMANENT', True )
            config.setdefault( 'SESSION_KEY_PREFIX', 'session:' )

            return FixedSqlAlchemySessionInterface(
                app, config['SESSION_SQLALCHEMY'],
                config['SESSION_SQLALCHEMY_TABLE'],
                config['SESSION_KEY_PREFIX'], config['SESSION_USE_SIGNER'],
                config['SESSION_PERMANENT'] )

@Lxstr
Copy link
Contributor

Lxstr commented Feb 2, 2024

Hi @rsyring @tassaron @behai-nguyen this repo is under new maintainers. Would be grateful if you can test 0.6.0 and confirm if this fixed your issues.

@Lxstr
Copy link
Contributor

Lxstr commented Feb 25, 2024

Table option keep_existing has been used to fix this and prevent accidental schema changes from 0.6.0

@Lxstr Lxstr closed this Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants