Skip to content

Commit

Permalink
PsqlDosMigrator: Commit changes when migrating existing schema
Browse files Browse the repository at this point in the history
There was a bug in the `PsqlDosMigrator.migrate` method where the final
step of migrating a database with a pre-existing schema to the latest
schema version, would not be committed. This would manifest itself if a
new database migration was added. When the user requested to migrate the
storage, the migration would be performed, but since it wasn't committed
as soon as the command terminated, the changes would be undone.

The fix is to explicitly commit the changes in `migrate` after the final
migration to the head version, by calling `Connection.commit`. A test is
added to explicitly test the pathway of migrating a database with a
pre-existing schema to the head version.
  • Loading branch information
sphuber committed May 15, 2023
1 parent c7a36fa commit f84fe5b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
1 change: 1 addition & 0 deletions aiida/storage/psql_dos/migrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ def migrate(self) -> None:
# finally migrate to the main head revision
MIGRATE_LOGGER.report('Migrating to the head of the main branch')
self.migrate_up('main@head')
self.connection.commit()

def migrate_up(self, version: str) -> None:
"""Migrate the database up to a specific version.
Expand Down
21 changes: 21 additions & 0 deletions tests/storage/psql_dos/migrations/test_all_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ def test_main(version, uninitialised_profile, reflect_schema, data_regression):
data_regression.check(reflect_schema(uninitialised_profile))


def test_main_initialized(uninitialised_profile):
"""Test that ``migrate`` properly stamps the new schema version when updating database with existing schema."""
migrator = PsqlDosMigrator(uninitialised_profile)

# Initialize database at first version of main branch
migrator.migrate_up('main@main_0001')
assert migrator.get_schema_version_profile(check_legacy=False) == 'main_0001'
migrator.close()

# Reinitialize the migrator to make sure we are fetching actual state of database and not in-memory state and then
# migrate to head schema version.
migrator = PsqlDosMigrator(uninitialised_profile)
migrator.migrate()
migrator.close()

# Reinitialize the migrator to make sure we are fetching actual state of database and not in-memory state and then
# assert that the database version is properly set to the head schema version
migrator = PsqlDosMigrator(uninitialised_profile)
assert migrator.get_schema_version_profile(check_legacy=False) == migrator.get_schema_version_head()


def test_head_vs_orm(uninitialised_profile, reflect_schema, data_regression):
"""Test that the migrations produce the same database schema as the models."""
migrator = PsqlDosMigrator(uninitialised_profile)
Expand Down

0 comments on commit f84fe5b

Please sign in to comment.