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

Flask-Migrate does not apply driver hacks that are applied by Flask-SQLAlchemy #276

Closed
fredreichbier opened this issue Jun 13, 2019 · 3 comments
Assignees
Labels

Comments

@fredreichbier
Copy link

fredreichbier commented Jun 13, 2019

First, thanks for this very useful module!

I think I stumbled upon a small inconsistency: I am using Flask-Migrate with a MySQL database. By default, Flask-SQLAlchemy applies some driver-specific hacks to database connections. More specifically, it sets the charset=utf8 option if no other charset has been explicitly set in case of a mysql driver.

https://github.com/pallets/flask-sqlalchemy/blob/419fa5ec65105866d3f0a1cf9ee6205dc69d6768/flask_sqlalchemy/__init__.py#L873

As Flask-Migrate does not seem to use the flask-sqlalchemy routines to set up connections, the same driver-specific hacks are not applied to the connection during migration.

I pushed a small example here (unfortunately it requires a MySQL database):

https://github.com/fredreichbier/flask-migrate-connections

If I run flask db upgrade, I get the following output:

Flask-SQLAlchemy db.session().bind = Engine(mysql+pymysql://test:***@192.168.33.101/test?charset=utf8)
Flask-SQLAlchemy character_set_results = ('utf8',)
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 0f14dff7473c, empty message
Migration URI = Engine(mysql+pymysql://test:***@192.168.33.101/test)
Migration character_set_results = ('utf8mb4',)

The output shows that the SQLAlchemy URIs used by Flask-SQLAlchemy and Flask-Migrate differ (mysql+pymysql://test:***@192.168.33.101/test?charset=utf8 vs mysql+pymysql://test:***@192.168.33.101/test), because Flask-SQLAlchemy adds a charset parameter while Flask-Migrate does not. Hence, the connections use different character_set_results settings.

In the case of utf8 vs utf8mb4, I guess this won't cause much trouble. But I have seen cases in which the migration connection falls back to character_set_results = latin1, which does cause problems if unicode data is processed in the migration script.

I am aware of #272 and indeed, explicitly passing charset=utf8 in SQLALCHEMY_DATABASE_URI fixes the issue. Nonetheless, I did not expect this behavior and hence thought I should probably report it :-)

@miguelgrinberg
Copy link
Owner

Yes, it's a valid point. I'll see what can be done. I think the env.py script will have to be modified so that it takes the SQLAlchemy engine object from Flask-SQLAlchemy.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Aug 3, 2019

@fredreichbier Can I ask you to give the master branch a try and confirm this looks good before I cut a new release? Thanks!

@fredreichbier
Copy link
Author

Hi @miguelgrinberg, I can confirm that on current master, Flask-SQLAlchemy and Flask-Migrate use the same engine with the same connection parameters:

Flask-SQLAlchemy character_set_results = ('utf8',)
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 0f14dff7473c, empty message
Migration URI = Engine(mysql+pymysql://test:***@192.168.33.101/test?charset=utf8)
Migration character_set_results = ('utf8',)

Thanks a lot!

zorun pushed a commit to zorun/ihatemoney that referenced this issue Apr 30, 2020
This fixes a potential issue with database connection that misses the
"charset=utf8" option during migrations: miguelgrinberg/Flask-Migrate#276
zorun pushed a commit to zorun/ihatemoney that referenced this issue May 21, 2020
This fixes a potential issue with database connection that misses the
"charset=utf8" option during migrations: miguelgrinberg/Flask-Migrate#276
zorun pushed a commit to spiral-project/ihatemoney that referenced this issue Jun 7, 2020
This fixes a potential issue with database connection that misses the
"charset=utf8" option during migrations: miguelgrinberg/Flask-Migrate#276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants