Flask-Migrate icon indicating copy to clipboard operation
Flask-Migrate copied to clipboard

Use cases for finer control over SQLALCHEMY_BINDS (compared to current --multidb flag)

Open tgross35 opened this issue 4 years ago • 4 comments

Problem overview

I would like to propose somehow being able to specify the specific bind(s) that Flask-Migrate migrates. Within SQLALCHEMY_BINDS there may be multiple databases; it is possible that Flask-Migrate doesn't have to handle all of them.

A related issue I have is that it migrations start to fail if there is no SQLALCHEMY_DATABASE_URI set for the app. This is acceptable in Flask if all databases are specified in SQLALCHEMY_BINDS, and all models include a __bind_key__. (This is actually preferred in my opinion for multiple binds - by forcing inclusion of a __bind_key__, it lowers risk of a model defaulting to the wrong SQLAlchemy engine and causing problems).

Solution Proposal

I would propose adding a syntax such as flask db init --bind=my_bind_1 --name=bind1, flask migrate bind1, and flask upgrade bind1. This would allow the user to address only needed binds - current behavior of --multidb could still exist for cases where all binds are handled at once.

Solution Notes

If this were implemented, it would make Flask-Migrate more applicable for the following use cases:

  • Multiple binds, some databases managed by another app / DBA outside of Flask
  • No use of SQLALCHEMY_DATABASE_URI for better code integrity/clarity
  • Ability to ignore local Sqlite databases that are created/destroyed at app setup/teardown
  • Allow usage of read-only database URIs where the user doesn't have create table access (for alembic_version table)

I'm curious if anyone else has run into similar issues - I have come across almost all of the use cases at least once.

A current workaround is just to comment out the BINDS item and all imports for unwanted database tracking, but this is not always straightforward.

tgross35 avatar Sep 28 '21 01:09 tgross35

You are making several suggestions here. Some I think are valid, while some others can be handled with a custom env.py file.

Multiple binds, some databases managed by another app / DBA outside of Flask

I think this is easily handled by modifying the env.py file, after flask init --multidb generates it. You can remove any binds that you do not want Flask-Migrate to handle there.

No use of SQLALCHEMY_DATABASE_URI for better code integrity/clarity

This one I agree. Flask-SQLAlchemy supports that, so Flask-Migrate should as well. I will look into that.

Ability to ignore local Sqlite databases that are created/destroyed at app setup/teardown

How is this different than the first item? You have a database that you want Flask-Migrate to skip, right? So the env.py can just remove the bind.

Allow usage of read-only database URIs where the user doesn't have create table access (for alembic_version table)

I'm not sure I understand this one. If you are saying that you want to manage migrations in a database where the alembic_version table cannot be written (i.e. this info needs to be written elsewhere), then this is a change that needs to be proposed to Alembic. If you want to skip these databases, then we are back at item 1, where you can tell env.py to skip any databases.

miguelgrinberg avatar Sep 28 '21 11:09 miguelgrinberg

Honestly, it did not even occur to me to edit env.py. That essentially solves the issue,

The four items you mentioned - I might not have been clear in my original post, but those are just four use cases I have come across that would be made easier by allowing migration for individual binds. They weren't meant as four unique problems. So yes, editing the env.py solves all of them.

While I would still vouch for some better integration with the command line (maybe the ability to create per-bind env files), it looks like this is pretty much a nonissue.

tgross35 avatar Sep 28 '21 15:09 tgross35

Hm, there seem to be further issues using a custom env.py. flask db heads, flask db show heads, flask db current and flask db show all show the same revision. However, flask db migrate insists that the target database is not up to date. stamp head doesn't change it at all.

Current workaround is to manually manage the revisions files, but this is tedious. I will dig more into where this problem might be coming from.

tgross35 avatar Oct 11 '21 22:10 tgross35

@tgross35 What you call a "custom env.py" is no different than a "stock env.py". My guess is that the difference is that the changes that you made to the env.py file are incorrect and are affecting the operating of Alembic.

miguelgrinberg avatar Oct 12 '21 21:10 miguelgrinberg

With using multiple databases, how is one to call stamp, current etc and specify the bind to use? There are no parameter options to these. When using upgrade and downgrade, I can pass in -x/-x-args and have a custom parameter to specify the binds I want to upgrade or downgrade.

I'm sure I'm doing something wrong. I manage one core database that is a different metadata, all those mapped classes specify bind_key as core. The other metadata does not specify a bind_key. This is a multi-tenant app. The tenants all have the same schema. I can't stamp a particular tenant database with a revision without manual update to alembic_version in that particular database(schema)

- Mike

xmikew avatar Sep 13 '23 16:09 xmikew

@xmikew this really is a question that applies to Alembic, not to Flask-Migrate, which just implements the integration between Flask, Flask-SQLAlchemy and Alembic.

Alembic does not natively support migrations for a subset of the databases, nor it supports multi-tenancy. These advanced usages can be implemented with a custom env.py file and the -x option if you need to pass custom arguments. If you Google for multi-tenancy in Alembic you may be able to find some examples of what people have implemented. There is also a recipe for multi-tenancy with Postgres schemas in the official Alembic docs from where you may be able to get some ideas.

miguelgrinberg avatar Sep 13 '23 16:09 miguelgrinberg

Thanks @miguelgrinberg. I do have an functional env.py that parses -x to get the tenant to upgrade/downgrade. However, seems like env.py is not loaded when you run some of the other commands (current etc).

Thanks much for the pointers!

- Mike

xmikew avatar Sep 14 '23 19:09 xmikew

Just to follow-up here. Alembic does allow -x to be passed to all commands. So if you run alembic -x db=testdb current the x args will be passed along to env.py. The parser for flask db does not appear to allow this except on upgrade and migrate.

xmikew avatar Sep 19 '23 14:09 xmikew

@xmikew I have made a small change to accept the -x argument in the db group instead of in the migrate and upgrade/downgrade commands. Please install the main branch of this repository to test. For example, try flask db -x db=testdb current.

miguelgrinberg avatar Sep 19 '23 15:09 miguelgrinberg

@miguelgrinberg With the release of 4.0.6, this is breaking with error:

Traceback (most recent call last):
  File "C:\pgadmin4\web\pgadmin\__init__.py", line 373, in backup_db_file
    db_upgrade(app)
  File "C:\pgadmin4\web\pgadmin\setup\db_upgrade.py", line 22, in db_upgrade
    flask_migrate.upgrade(migration_folder)
  File "C:\venv\pypg312\Lib\site-packages\flask_migrate\__init__.py", line 111, in wrapped
    f(*args, **kwargs)
  File "C:\venv\pypg312\Lib\site-packages\flask_migrate\__init__.py", line 198, in upgrade
    config = current_app.extensions['migrate'].migrate.get_config(directory,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\venv\pypg312\Lib\site-packages\flask_migrate\__init__.py", line 96, in get_config
    for x in g.x_arg:
             ^^^^^^^
  File "C:\venv\pypg312\Lib\site-packages\flask\ctx.py", line 54, in __getattr__
    raise AttributeError(name) from None
AttributeError: x_arg

adityatoshniwal avatar Mar 11 '24 07:03 adityatoshniwal

I can confirm the problem with pgAdmin4 7.8. The update from 4.0.5 to 4.0.6 breaks it as shown above @miguelgrinberg @adityatoshniwal

hfp-ak avatar Mar 11 '24 18:03 hfp-ak

Fix is released. Please report back if there are any remaining problems.

miguelgrinberg avatar Mar 11 '24 18:03 miguelgrinberg

@miguelgrinberg Thank you for the quick fix. I can confirm the error is gone.

adityatoshniwal avatar Mar 12 '24 04:03 adityatoshniwal