-
Notifications
You must be signed in to change notification settings - Fork 191
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
SqliteDosStorage
: Make the migrator compatible with SQLite
#6429
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
"""Module with common resources related to storage migrations.""" | ||
|
||
TEMPLATE_INVALID_SCHEMA_VERSION = """ | ||
Database schema version `{schema_version_database}` is incompatible with the required schema version `{schema_version_code}`. | ||
To migrate the database schema version to the current one, run the following command: | ||
|
||
verdi -p {profile_name} storage migrate | ||
""" # noqa: E501 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
########################################################################### | ||
# Copyright (c), The AiiDA team. All rights reserved. # | ||
# This file is part of the AiiDA code. # | ||
# # | ||
# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # | ||
# For further information on the license, see the LICENSE.txt file # | ||
# For further information please visit http://www.aiida.net # | ||
########################################################################### | ||
"""Environment configuration to be used by alembic to perform database migrations.""" | ||
|
||
from alembic import context | ||
|
||
|
||
def run_migrations_online(): | ||
"""Run migrations in 'online' mode. | ||
|
||
The connection should have been passed to the config, which we use to configure the migration context. | ||
""" | ||
from aiida.storage.sqlite_zip.models import SqliteBase | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really should extract SqliteBase and related out of sqlite_zip to something like |
||
|
||
config = context.config | ||
|
||
connection = config.attributes.get('connection', None) | ||
aiida_profile = config.attributes.get('aiida_profile', None) | ||
on_version_apply = config.attributes.get('on_version_apply', None) | ||
|
||
if connection is None: | ||
from aiida.common.exceptions import ConfigurationError | ||
|
||
raise ConfigurationError('An initialized connection is expected for the AiiDA online migrations.') | ||
if aiida_profile is None: | ||
from aiida.common.exceptions import ConfigurationError | ||
|
||
raise ConfigurationError('An aiida_profile is expected for the AiiDA online migrations.') | ||
|
||
context.configure( | ||
connection=connection, | ||
target_metadata=SqliteBase.metadata, | ||
transaction_per_migration=True, | ||
aiida_profile=aiida_profile, | ||
on_version_apply=on_version_apply, | ||
) | ||
|
||
context.run_migrations() | ||
|
||
|
||
try: | ||
if context.is_offline_mode(): | ||
raise NotImplementedError('This feature is not currently supported.') | ||
|
||
run_migrations_online() | ||
except NameError: | ||
# This will occur in an environment that is just compiling the documentation | ||
pass |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to somewhere include a high-level description of how is the SqliteDosMigrator different from its base implementation (based off PsqlDosMigrator) i.e. what you have written up on the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not so much that the mechanism is different, it is just that it provides a separate set of actual migrations, which will have to be sqlite or psql specific since they can use direct SQL features that are only supported by one or the other. Before, all plugins used the same migrations which were written for PSQL and so failed for sqlite databases.
Only other difference is that the new sqlite-migrator does not have the complicated additional logic to deal with migrating from legacy Django databases as this was introduced after Django had already been dropped and there were just sqlalchemy databases. I will add something to the docstring.