flask-sqlalchemy
flask-sqlalchemy copied to clipboard
after_create event is called multiple times for a single MetaData
Expected Behavior
When calling db.create_all()
, the after_create
event should only be dispatched once for a sqlalchemy.MetaData
object when using multiple binds.
Workaround-ish: There should be a way (extra keyword maybe?) to detect for which current bind_key
the event is called so that the listener can decide whether to run. The only way to do this right now is via kw["tables"][0].info.get("bind_key")
which won't work if there aren't any tables, yet.
Actual Behavior
It's dispatched multiple times (once per bind).
Environment
- Python version: 3.6
- Flask-SQLAlchemy version: 2.4
- SQLAlchemy version: 1.3.13
I think this is handled inside SQLAlchemy itself. In the short term, something like your workaround looks like it may work?
Surely after_create
would make the tables you need to get the bind key? Or, could you catch the exception raised when they don't exist, to skip your logic in the cases when it's not needed/won't run?
I think this is handled inside SQLAlchemy itself.
I might be wrong here, but my understanding is that multi-db support via bind-keys is added by flask_sqlalchemy
as documented here. The deliberate re-dispatch happens because of the for-loop in _execute_for_all_tables
in flask_sqlalchemy/__init__.py
:
for bind in binds:
extra = {}
if not skip_tables:
tables = self.get_tables_for_bind(bind)
extra['tables'] = tables
op = getattr(self.Model.metadata, operation)
op(bind=self.get_engine(app, bind), **extra)
Surely after_create would make the tables you need to get the bind key?
Here's an example where this doesn't work well: The sqlalchemy_utils
library has a create_materialized_view
helper, which will listen for after_create
to then create the materialized view. What I've been seeing is that because of the double event dispatch, the library will create views unintentionally in both databases. It ultimately fails because the tables, that the view references, don't exist in the second database.
Or, could you catch the exception raised when they don't exist, to skip your logic in the cases when it's not needed/won't run?
I'm worried that I'd make that catch too broad (especially because the exceptions will be low-level DB exceptions), swallowing an exception which shouldn't have been.
In the short term, something like your workaround looks like it may work?
I'll probably monkey-patch flask_sqlalchemy.SQLAlchemy.get_engine
to setattr(engine, "bind_key", bind)
- this seems to be more resilient. Is that maybe something that I could create a PR for and try to merge upstream?
You're quite correct - sorry for my misunderstanding :)
I think a PR would be the best way to resolve this, but I think there are some considerations. We need to figure out if this is a desirable behaviour or just an oversight. I think it there may be many use-cases whereby multiple events are desirable.
Perhaps @mitsuhiko or @davidism can advise further.
#1087 uses a separate metadata per bind, so this should no longer be an issue.