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

Add support for multiple databases migration #70

Merged
merged 1 commit into from
Aug 1, 2015

Conversation

ncloudioj
Copy link
Contributor

This PR is aimed to partially fix #20, it uses "multidb" template of Alembic. Flask-Migrate users, however, still need to change the alembic.ini and env.py to provide the required database meta information.

@miguelgrinberg
Copy link
Owner

Thanks. I think I'm going to be able to make the edits to the config files automatic, all on top of your patch. I'll delay the merge a couple days until I can have that fully figured out.

@miguelgrinberg
Copy link
Owner

@ncloudioj any reason why you did not follow the recommended format for binds in the test application? You created two sqlalchemy objects, instead of using a single one and assigning tables to each bind with the bind_key argument.

@ncloudioj
Copy link
Contributor Author

@miguelgrinberg It has to do with the "metadata" in "env.py". Again, have tried reusing a single db instance, but Alembic only created schema for the main db. Seems like Alembic needs different "metadata" for different database.

@miguelgrinberg
Copy link
Owner

Unfortunately I think that is going to be a deal killer for some. Is it even possible to set up a relationship across databases with this model?

In any case, I don't remember the exact structure, but Flask-SQLAlchemy keeps the databases and their metadata internally, so I think it should be possible to get them using the recommended configuration.

@ncloudioj
Copy link
Contributor Author

Yes, I agree that it's annoying. But unfortunately Alembic needs metadata for each db engine. Well, will take another look to see if any improvement exists.

@miguelgrinberg
Copy link
Owner

@ncloudioj I'll do some research on this as well. SQLAlchemy has the support for binds, so we need to figure out how Alembic can use that.

@miguelgrinberg
Copy link
Owner

@ncloudioj I'm merging your patch, and right after I'm putting my changes to make it fully configuration free. Thanks!

miguelgrinberg added a commit that referenced this pull request Aug 1, 2015
Add support for multiple databases migration
@miguelgrinberg miguelgrinberg merged commit d7ceeea into miguelgrinberg:master Aug 1, 2015
@ncloudioj
Copy link
Contributor Author

@miguelgrinberg Awesome! Apparently, way more reasonable than my hacky one. Well done.

We recently introduced another database in production. Also, we'd like to keep the amazing Flask-Migrate as our database migration tool. Finally, we can have both. Thanks.

@miguelgrinberg
Copy link
Owner

@ncloudioj Check the changes I've made to the test application. The way you configured the bind for the second table was incorrect, and caused the table to go into the main database. That made me realize that the unit tests were insufficient for multidb, I needed to also ensure that the tables were going to the correct databases. All in all it is a great addition to the project, thanks!

@espositocode
Copy link

This is amazing. Thanks!

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for multiple databases
3 participants