diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f27bd6db64..ec32100996 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -78,9 +78,6 @@ aiida/backends/sqlalchemy/queries.py| aiida/backends/sqlalchemy/tests/generic.py| aiida/backends/sqlalchemy/tests/__init__.py| - aiida/backends/sqlalchemy/tests/migrations.py| - aiida/backends/sqlalchemy/tests/migration_test/versions/470e57bc0936_create_account_table.py| - aiida/backends/sqlalchemy/tests/migration_test/versions/b947a8821295_add_a_column.py| aiida/backends/sqlalchemy/tests/nodes.py| aiida/backends/sqlalchemy/tests/query.py| aiida/backends/sqlalchemy/tests/schema.py| diff --git a/aiida/backends/sqlalchemy/tests/migration_test/__init__.py b/aiida/backends/sqlalchemy/tests/migration_test/__init__.py deleted file mode 100644 index a7e3fad50c..0000000000 --- a/aiida/backends/sqlalchemy/tests/migration_test/__init__.py +++ /dev/null @@ -1,9 +0,0 @@ -# -*- coding: utf-8 -*- -########################################################################### -# 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 # -########################################################################### diff --git a/aiida/backends/sqlalchemy/tests/migration_test/versions/470e57bc0936_create_account_table.py b/aiida/backends/sqlalchemy/tests/migration_test/versions/470e57bc0936_create_account_table.py deleted file mode 100644 index d74538f11d..0000000000 --- a/aiida/backends/sqlalchemy/tests/migration_test/versions/470e57bc0936_create_account_table.py +++ /dev/null @@ -1,40 +0,0 @@ -# -*- coding: utf-8 -*- -########################################################################### -# 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 # -########################################################################### -"""create account table - -Revision ID: 470e57bc0936 -Revises: -Create Date: 2017-06-16 17:53:10.920345 - -""" -from __future__ import division -from __future__ import print_function -from __future__ import absolute_import -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '470e57bc0936' -down_revision = None -branch_labels = None -depends_on = None - - -def upgrade(): - op.create_table( - 'account', - sa.Column('id', sa.Integer, primary_key=True), - sa.Column('name', sa.String(50), nullable=False), - sa.Column('description', sa.Unicode(200)), - ) - -def downgrade(): - op.drop_table('account') diff --git a/aiida/backends/sqlalchemy/tests/migration_test/versions/__init__.py b/aiida/backends/sqlalchemy/tests/migration_test/versions/__init__.py deleted file mode 100644 index a7e3fad50c..0000000000 --- a/aiida/backends/sqlalchemy/tests/migration_test/versions/__init__.py +++ /dev/null @@ -1,9 +0,0 @@ -# -*- coding: utf-8 -*- -########################################################################### -# 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 # -########################################################################### diff --git a/aiida/backends/sqlalchemy/tests/migration_test/versions/b947a8821295_add_a_column.py b/aiida/backends/sqlalchemy/tests/migration_test/versions/b947a8821295_add_a_column.py deleted file mode 100644 index 1dfd68292c..0000000000 --- a/aiida/backends/sqlalchemy/tests/migration_test/versions/b947a8821295_add_a_column.py +++ /dev/null @@ -1,35 +0,0 @@ -# -*- coding: utf-8 -*- -########################################################################### -# 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 # -########################################################################### -"""Add a column - -Revision ID: b947a8821295 -Revises: 470e57bc0936 -Create Date: 2017-06-16 18:09:03.152982 - -""" -from __future__ import division -from __future__ import print_function -from __future__ import absolute_import -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = 'b947a8821295' -down_revision = '470e57bc0936' -branch_labels = None -depends_on = None - - -def upgrade(): - op.add_column('account', sa.Column('last_transaction_date', sa.DateTime)) - -def downgrade(): - op.drop_column('account', 'last_transaction_date') diff --git a/aiida/backends/sqlalchemy/tests/migrations.py b/aiida/backends/sqlalchemy/tests/migrations.py index eaf6b716be..a6c5767148 100644 --- a/aiida/backends/sqlalchemy/tests/migrations.py +++ b/aiida/backends/sqlalchemy/tests/migrations.py @@ -7,6 +7,10 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### +""" +Tests for the migration engine (Alembic) as well as for the AiiDA migrations +for SQLAlchemy. +""" from __future__ import division from __future__ import absolute_import from __future__ import print_function @@ -21,17 +25,10 @@ from aiida.backends.sqlalchemy import utils from aiida.backends.sqlalchemy.models.base import Base from aiida.backends.sqlalchemy.tests.utils import new_database -from aiida.backends.sqlalchemy.utils import get_migration_head, get_db_schema_version from aiida.backends.testbase import AiidaTestCase -alembic_root = os.path.join(os.path.dirname(__file__), 'migrations', 'alembic') - - -TEST_ALEMBIC_REL_PATH = 'migration_test' - - -class TestMigrationApplicationSQLA(AiidaTestCase): +class TestMigrationsSQLA(AiidaTestCase): """ This class contains tests for the migration mechanism of SQLAlchemy called alembic. It checks if the migrations can be applied and removed correctly. @@ -43,114 +40,215 @@ class TestMigrationApplicationSQLA(AiidaTestCase): # the testing) alembic_dpath = None + migrate_from = None + migrate_to = None + @classmethod def setUpClass(cls, *args, **kwargs): - super(TestMigrationApplicationSQLA, cls).setUpClass(*args, **kwargs) - cls.migr_method_dir_path = os.path.dirname( - os.path.realpath(utils.__file__)) + """ + Prepare the test class with the alembivc configuration + """ + super(TestMigrationsSQLA, cls).setUpClass(*args, **kwargs) + cls.migr_method_dir_path = os.path.dirname(os.path.realpath(utils.__file__)) + alembic_dpath = os.path.join(cls.migr_method_dir_path, utils.ALEMBIC_REL_PATH) + cls.alembic_cfg = Config() + cls.alembic_cfg.set_main_option('script_location', alembic_dpath) def setUp(self): - self.migrate_db_with_non_testing_migrations("base") + """ + Go to the migrate_from revision, applhy setUpBeforeMigration, then + run the migration. + """ + super(TestMigrationsSQLA, self).setUp() + from aiida.orm import autogroup - def tearDown(self): - self.migrate_db_with_non_testing_migrations("head") + self.current_autogroup = autogroup.current_autogroup + autogroup.current_autogroup = None + assert self.migrate_from and self.migrate_to, \ + "TestCase '{}' must define migrate_from and migrate_to properties".format(type(self).__name__) - def migrate_db_with_non_testing_migrations(self, destination): - if destination not in ["head", "base"]: - raise TypeError("Only head & base are accepted as destination " - "values.") - # Set the alembic script directory location - self.alembic_dpath = os.path.join(self.migr_method_dir_path, - utils.ALEMBIC_REL_PATH) - alembic_cfg = Config() - alembic_cfg.set_main_option('script_location', self.alembic_dpath) + try: + self.migrate_db_down(self.migrate_from) + self.setUpBeforeMigration() + self.migrate_db_up(self.migrate_to) + except Exception: + # Bring back the DB to the correct state if this setup part fails + self._reset_database_and_schema() + raise + + def migrate_db_up(self, destination): + """ + Perform a migration upwards (upgrade) with alembic + :param destination: the name of the destination migration + """ # Undo all previous real migration of the database with sa.engine.begin() as connection: - alembic_cfg.attributes['connection'] = connection - if destination == "head": - command.upgrade(alembic_cfg, "head") - else: - command.downgrade(alembic_cfg, "base") + self.alembic_cfg.attributes['connection'] = connection # pylint: disable=unsupported-assignment-operation + command.upgrade(self.alembic_cfg, destination) + + def migrate_db_down(self, destination): + """ + Perform a migration downwards (downgrade) with alembic + + :param destination: the name of the destination migration + """ + with sa.engine.begin() as connection: + self.alembic_cfg.attributes['connection'] = connection # pylint: disable=unsupported-assignment-operation + command.downgrade(self.alembic_cfg, destination) + + def tearDown(self): + """ + Resets both the database content and the schema to prepare for the + next test + """ + from aiida.orm import autogroup + self._reset_database_and_schema() + autogroup.current_autogroup = self.current_autogroup + super(TestMigrationsSQLA, self).tearDown() + + def setUpBeforeMigration(self): # pylint: disable=invalid-name + """ + Anything to do before running the migrations. + This is typically implemented in test subclasses. + """ - def test_migrations_forward_backward(self): + def _reset_database_and_schema(self): """ - This is a very broad test that checks that the migration mechanism - works. More specifically, it checks that:: + Bring back the DB to the correct state. - - Alembic database migrations to specific versions work (upgrade & - downgrade) + It is important to also reset the database content to avoid hanging + of tests. + """ + self.reset_database() + self.migrate_db_up("head") - - The methods that are checking the database schema version and perform - the migration procedure to the last version work correctly. + @property + def current_rev(self): + """ + Utility method to get the current revision string + """ + from alembic.migration import MigrationContext # pylint: disable=import-error + with sa.engine.begin() as connection: + context = MigrationContext.configure(connection) + current_rev = context.get_current_revision() + return current_rev + @staticmethod + def get_auto_base(): """ - from aiida.backends.sqlalchemy.tests.migration_test import versions - from aiida.backends.sqlalchemy.utils import migrate_database + Return the automap_base class that automatically inspects the current + database and return SQLAlchemy Models. - try: - # Constructing the versions directory - versions_dpath = os.path.join( - os.path.dirname(versions.__file__)) - - # Setting dynamically the the path to the alembic configuration - # (this is where the env.py file can be found) - alembic_cfg = Config() - alembic_cfg.set_main_option('script_location', self.alembic_dpath) - # Setting dynamically the versions directory. These are the - # migration scripts to pass from one version to the other. The - # default ones are overridden with test-specific migrations. - alembic_cfg.set_main_option('version_locations', versions_dpath) - - # Using the connection initialized by the tests - with sa.engine.begin() as connection: - alembic_cfg.attributes['connection'] = connection - - self.assertIsNone(get_db_schema_version(alembic_cfg), - "The initial database version should be " - "None (no version) since the test setUp " - "method should undo all migrations") - # Migrate the database to the latest version - migrate_database(alembic_cfg=alembic_cfg) - with sa.engine.begin() as connection: - alembic_cfg.attributes['connection'] = connection - self.assertEquals(get_db_schema_version(alembic_cfg), - get_migration_head(alembic_cfg), - "The latest database version is not the " - "expected one.") - with sa.engine.begin() as connection: - alembic_cfg.attributes['connection'] = connection - # Migrating the database to the base version - command.downgrade(alembic_cfg, "base") - self.assertIsNone(get_db_schema_version(alembic_cfg), - "The database version is not the expected " - "one. It should be None (initial).") - - except Exception as test_ex: - # If there is an exception, clean the alembic related tables - from sqlalchemy.engine import reflection - - # Getting the current database table names - inspector = reflection.Inspector.from_engine( - sa.get_scoped_session().bind) - db_table_names = set(inspector.get_table_names()) - # The alembic related database names - alemb_table_names = set(['account', 'alembic_version']) - - # Get the intersection of the above tables - tables_to_drop = set.intersection(db_table_names, - alemb_table_names) - # Delete only the tables that exist - for table in tables_to_drop: - try: - with sa.engine.begin() as connection: - connection.execute('DROP TABLE {};'.format(table)) - except Exception as db_ex: - print("The following error occured during the cleaning of" - "the database: {}".format(db_ex)) - # Since the database cleaning is over, raise the test - # exception that was caught - raise test_ex + Note that these are NOT the ones in AiiDA SQLAlchemy models, so do not + have the special methods that we define there (like .save()). + """ + from alembic.migration import MigrationContext # pylint: disable=import-error + from sqlalchemy.ext.automap import automap_base # pylint: disable=import-error,no-name-in-module + + with sa.engine.begin() as connection: + context = MigrationContext.configure(connection) + bind = context.bind + + base = automap_base() + # reflect the tables + base.prepare(bind.engine, reflect=True) + + return base + + +class TestMigrationEngine(TestMigrationsSQLA): + """ + Just a simple test to verify that the TestMigrationsSQLA class indeed + works and moves between the expected migration revisions + """ + migrate_from = 'b8b23ddefad4' # b8b23ddefad4_dbgroup_name_to_label_type_to_type_string.py + migrate_to = 'e72ad251bcdb' # e72ad251bcdb_dbgroup_class_change_type_string_values.py + + def setUpBeforeMigration(self): + """ + Cache the start revision + """ + self.start_revision = self.current_rev + + def test_revision_numbers(self): + """ + Check that we went to the correct version + """ + self.assertEqual(self.start_revision, self.migrate_from) + self.assertEqual(self.current_rev, self.migrate_to) + + +class TestGroupRenamingMigration(TestMigrationsSQLA): + """ + Test the migration that renames the DbGroup type strings + """ + # b8b23ddefad4_dbgroup_name_to_label_type_to_type_string.py + # e72ad251bcdb_dbgroup_class_change_type_string_values.py + migrate_from = 'b8b23ddefad4' # b8b23ddefad4_dbgroup_name_to_label_type_to_type_string.py + migrate_to = 'e72ad251bcdb' # e72ad251bcdb_dbgroup_class_change_type_string_values.py + + def setUpBeforeMigration(self): + """ + Create the DbGroups with the old type strings + """ + from sqlalchemy.orm import Session # pylint: disable=import-error,no-name-in-module + + # Create group + DbGroup = self.get_auto_base().classes.db_dbgroup # pylint: disable=invalid-name + DbUser = self.get_auto_base().classes.db_dbuser # pylint: disable=invalid-name + + with sa.engine.begin() as connection: + session = Session(connection.engine) + default_user = DbUser(is_superuser=False, email="{}@aiida.net".format(self.id())) + session.add(default_user) + session.commit() + + # test user group type_string: '' -> 'user' + group_user = DbGroup(label='test_user_group', user_id=default_user.id, type_string='') + session.add(group_user) + # test data.upf group type_string: 'data.upf.family' -> 'data.upf' + group_data_upf = DbGroup( + label='test_data_upf_group', user_id=default_user.id, type_string='data.upf.family') + session.add(group_data_upf) + # test auto.import group type_string: 'aiida.import' -> 'auto.import' + group_autoimport = DbGroup(label='test_import_group', user_id=default_user.id, type_string='aiida.import') + session.add(group_autoimport) + # test auto.run group type_string: 'autogroup.run' -> 'auto.run' + group_autorun = DbGroup(label='test_autorun_group', user_id=default_user.id, type_string='autogroup.run') + session.add(group_autorun) + + session.commit() + self.group_user_pk = group_user.id + self.group_data_upf_pk = group_data_upf.id + self.group_autoimport_pk = group_autoimport.id + self.group_autorun_pk = group_autorun.id + + def test_group_string_update(self): + """ + Test that the type strings are properly migrated + """ + from sqlalchemy.orm import Session # pylint: disable=import-error,no-name-in-module + DbGroup = self.get_auto_base().classes.db_dbgroup # pylint: disable=invalid-name + + with sa.engine.begin() as connection: + session = Session(connection.engine) + + # test user group type_string: '' -> 'user' + group_user = session.query(DbGroup).filter(DbGroup.id == self.group_user_pk).one() + self.assertEqual(group_user.type_string, 'user') + + # test data.upf group type_string: 'data.upf.family' -> 'data.upf' + group_data_upf = session.query(DbGroup).filter(DbGroup.id == self.group_data_upf_pk).one() + self.assertEqual(group_data_upf.type_string, 'data.upf') + + # test auto.import group type_string: 'aiida.import' -> 'auto.import' + group_autoimport = session.query(DbGroup).filter(DbGroup.id == self.group_autoimport_pk).one() + self.assertEqual(group_autoimport.type_string, 'auto.import') + + # test auto.run group type_string: 'autogroup.run' -> 'auto.run' + group_autorun = session.query(DbGroup).filter(DbGroup.id == self.group_autorun_pk).one() + self.assertEqual(group_autorun.type_string, 'auto.run') class TestMigrationSchemaVsModelsSchema(unittest.TestCase): @@ -178,15 +276,12 @@ def setUp(self): from sqlalchemydiff.util import get_temporary_uri from aiida.backends.sqlalchemy.migrations import versions - self.migr_method_dir_path = os.path.dirname( - os.path.realpath(utils.__file__)) + self.migr_method_dir_path = os.path.dirname(os.path.realpath(utils.__file__)) # Set the alembic script directory location - self.alembic_dpath = os.path.join(self.migr_method_dir_path, - utils.ALEMBIC_REL_PATH) + self.alembic_dpath = os.path.join(self.migr_method_dir_path, utils.ALEMBIC_REL_PATH) # Constructing the versions directory - versions_dpath = os.path.join( - os.path.dirname(versions.__file__)) + versions_dpath = os.path.join(os.path.dirname(versions.__file__)) # Setting dynamically the the path to the alembic configuration # (this is where the env.py file can be found) @@ -206,8 +301,7 @@ def setUp(self): self.db_url_right = get_temporary_uri(str(curr_db_url)) # Put the correct database url to the database used by alembic - self.alembic_cfg_left.set_main_option("sqlalchemy.url", - self.db_url_left) + self.alembic_cfg_left.set_main_option("sqlalchemy.url", self.db_url_left) # Database creation new_database(self.db_url_left) @@ -218,28 +312,25 @@ def tearDown(self): destroy_database(self.db_url_left) destroy_database(self.db_url_right) - def test_model_and_migration_schemas_are_the_same(self): + def test_model_and_migration_schemas_are_the_same(self): # pylint: disable=invalid-name """Compare two databases. Compares the database obtained with all migrations against the one we get out of the models. It produces a text file with the results to help debug differences. """ - from sqlalchemy.engine import create_engine + from sqlalchemy.engine import create_engine # pylint: disable=import-error,no-name-in-module from sqlalchemydiff import compare with create_engine(self.db_url_left).begin() as connection: - self.alembic_cfg_left.attributes['connection'] = connection + self.alembic_cfg_left.attributes['connection'] = connection # pylint: disable=unsupported-assignment-operation command.upgrade(self.alembic_cfg_left, "head") engine_right = create_engine(self.db_url_right) Base.metadata.create_all(engine_right) engine_right.dispose() - result = compare( - self.db_url_left, self.db_url_right, set(['alembic_version'])) + result = compare(self.db_url_left, self.db_url_right, set(['alembic_version'])) - self.assertTrue(result.is_match, - "The migration database doesn't match to the one " - "created by the models.\nDifferences: " + - result._dump_data(result.errors)) + self.assertTrue(result.is_match, "The migration database doesn't match to the one " + "created by the models.\nDifferences: " + result._dump_data(result.errors)) # pylint: disable=protected-access diff --git a/aiida/common/log.py b/aiida/common/log.py index 68af0327cc..cc10860831 100644 --- a/aiida/common/log.py +++ b/aiida/common/log.py @@ -175,7 +175,7 @@ def evaluate_logging_configuration(dictionary): for key, value in dictionary.items(): if isinstance(value, collections.Mapping): result[key] = evaluate_logging_configuration(value) - elif isinstance(value, types.LambdaType): + elif isinstance(value, types.LambdaType): # pylint: disable=no-member result[key] = value() else: result[key] = value diff --git a/docs/source/developer_guide/core/modifying_the_schema.rst b/docs/source/developer_guide/core/modifying_the_schema.rst index b4366e70fc..0e74913663 100644 --- a/docs/source/developer_guide/core/modifying_the_schema.rst +++ b/docs/source/developer_guide/core/modifying_the_schema.rst @@ -186,6 +186,10 @@ If you need to change the database schema follow these steps: as you run your first verdi command. You can also migrate it manually with the help of the alembic_manage.py script as you can see below. +5. Prepare tests for your migrations. An example of a test can be found here: + ``aiida_core/aiida/backends/sqlalchemy/tests/migrations.py`` + + Overview of alembic_manage.py commands ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The alembic_manage.py provides several options to control your SQLAlchemy