From 5c6d1f37c521c245f7f1bfb53d9f90813f1e521e Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Thu, 15 Sep 2016 18:27:28 +0000 Subject: [PATCH] Rename sql.migration_helpers to sql.upgrades "Helpers" is one of those terms like "utils" that means "we don't know what belongs in this module and what doesn't," so let's get rid of it in favor of a shorter name that better reflects the purpose of everything in the module. This name nicely opposes to sql.core, which contains everything we use for more general database interactions. Change-Id: I65ed77bc597b66eaffe5126275366f8ae141524b --- keystone/cmd/cli.py | 12 ++++---- ...unencrypted_blob_column_from_credential.py | 4 +-- ...y_hash_and_encrypted_blob_to_credential.py | 4 +-- .../versions/081_add_endpoint_policy_table.py | 4 +-- .../versions/082_add_federation_tables.py | 4 +-- .../versions/083_add_oauth1_tables.py | 4 +-- .../versions/084_add_revoke_tables.py | 4 +-- .../085_add_endpoint_filtering_table.py | 4 +-- .../093_migrate_domains_to_projects.py | 6 ++-- .../sql/{migration_helpers.py => upgrades.py} | 0 keystone/tests/unit/test_cli.py | 26 ++++++++--------- .../tests/unit/test_sql_banned_operations.py | 6 ++-- keystone/tests/unit/test_sql_upgrade.py | 28 +++++++++---------- 13 files changed, 53 insertions(+), 53 deletions(-) rename keystone/common/sql/{migration_helpers.py => upgrades.py} (100%) diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index 6c9a1b7e3..25a8ff8f5 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -29,7 +29,7 @@ from keystone.cmd import doctor from keystone.common import driver_hints from keystone.common import openssl from keystone.common import sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades from keystone.common import utils import keystone.conf from keystone.credential.providers import fernet as credential_fernet @@ -435,13 +435,13 @@ class DbSync(BaseApp): assert_not_extension(CONF.command.extension) if CONF.command.expand: - migration_helpers.expand_schema() + upgrades.expand_schema() elif CONF.command.migrate: - migration_helpers.migrate_data() + upgrades.migrate_data() elif CONF.command.contract: - migration_helpers.contract_schema() + upgrades.contract_schema() else: - migration_helpers.offline_sync_database_to_version( + upgrades.offline_sync_database_to_version( CONF.command.version) @@ -464,7 +464,7 @@ class DbVersion(BaseApp): @staticmethod def main(): assert_not_extension(CONF.command.extension) - print(migration_helpers.get_db_version()) + print(upgrades.get_db_version()) class BasePermissionsSetup(BaseApp): diff --git a/keystone/common/sql/contract_repo/versions/003_remove_unencrypted_blob_column_from_credential.py b/keystone/common/sql/contract_repo/versions/003_remove_unencrypted_blob_column_from_credential.py index b247e4fd7..3c169e644 100644 --- a/keystone/common/sql/contract_repo/versions/003_remove_unencrypted_blob_column_from_credential.py +++ b/keystone/common/sql/contract_repo/versions/003_remove_unencrypted_blob_column_from_credential.py @@ -10,7 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades import sqlalchemy as sql @@ -22,7 +22,7 @@ def upgrade(migrate_engine): credential_table = sql.Table('credential', meta, autoload=True) credential_table.c.blob.drop() - if migration_helpers.USE_TRIGGERS: + if upgrades.USE_TRIGGERS: if migrate_engine.name == 'postgresql': drop_credential_update_trigger = ( 'DROP TRIGGER credential_update_read_only on credential;' diff --git a/keystone/common/sql/expand_repo/versions/003_add_key_hash_and_encrypted_blob_to_credential.py b/keystone/common/sql/expand_repo/versions/003_add_key_hash_and_encrypted_blob_to_credential.py index a245c677f..3e9f25b03 100644 --- a/keystone/common/sql/expand_repo/versions/003_add_key_hash_and_encrypted_blob_to_credential.py +++ b/keystone/common/sql/expand_repo/versions/003_add_key_hash_and_encrypted_blob_to_credential.py @@ -13,7 +13,7 @@ import sqlalchemy as sql from keystone.common import sql as ks_sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades # NOTE(lbragstad): MySQL error state of 45000 is a generic unhandled exception. @@ -105,7 +105,7 @@ def upgrade(migrate_engine): credential_table.create_column(encrypted_blob) credential_table.c.blob.alter(nullable=True) - if not migration_helpers.USE_TRIGGERS: + if not upgrades.USE_TRIGGERS: # Skip managing triggers if we're doing an offline upgrade. return diff --git a/keystone/common/sql/migrate_repo/versions/081_add_endpoint_policy_table.py b/keystone/common/sql/migrate_repo/versions/081_add_endpoint_policy_table.py index a0c307d08..f8bffa1cd 100644 --- a/keystone/common/sql/migrate_repo/versions/081_add_endpoint_policy_table.py +++ b/keystone/common/sql/migrate_repo/versions/081_add_endpoint_policy_table.py @@ -14,12 +14,12 @@ import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades def upgrade(migrate_engine): try: - extension_version = migration_helpers.get_db_version( + extension_version = upgrades.get_db_version( extension='endpoint_policy', engine=migrate_engine) except Exception: diff --git a/keystone/common/sql/migrate_repo/versions/082_add_federation_tables.py b/keystone/common/sql/migrate_repo/versions/082_add_federation_tables.py index f0b18048a..bbda554a4 100644 --- a/keystone/common/sql/migrate_repo/versions/082_add_federation_tables.py +++ b/keystone/common/sql/migrate_repo/versions/082_add_federation_tables.py @@ -14,7 +14,7 @@ import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades import keystone.conf CONF = keystone.conf.CONF @@ -23,7 +23,7 @@ _RELAY_STATE_PREFIX = 'relay_state_prefix' def upgrade(migrate_engine): try: - extension_version = migration_helpers.get_db_version( + extension_version = upgrades.get_db_version( extension='federation', engine=migrate_engine) except Exception: diff --git a/keystone/common/sql/migrate_repo/versions/083_add_oauth1_tables.py b/keystone/common/sql/migrate_repo/versions/083_add_oauth1_tables.py index 5a859b4b2..8bbccb962 100644 --- a/keystone/common/sql/migrate_repo/versions/083_add_oauth1_tables.py +++ b/keystone/common/sql/migrate_repo/versions/083_add_oauth1_tables.py @@ -14,12 +14,12 @@ import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades def upgrade(migrate_engine): try: - extension_version = migration_helpers.get_db_version( + extension_version = upgrades.get_db_version( extension='oauth1', engine=migrate_engine) except Exception: diff --git a/keystone/common/sql/migrate_repo/versions/084_add_revoke_tables.py b/keystone/common/sql/migrate_repo/versions/084_add_revoke_tables.py index 1a28a53cc..0cf37e7d7 100644 --- a/keystone/common/sql/migrate_repo/versions/084_add_revoke_tables.py +++ b/keystone/common/sql/migrate_repo/versions/084_add_revoke_tables.py @@ -14,12 +14,12 @@ import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades def upgrade(migrate_engine): try: - extension_version = migration_helpers.get_db_version( + extension_version = upgrades.get_db_version( extension='revoke', engine=migrate_engine) except Exception: diff --git a/keystone/common/sql/migrate_repo/versions/085_add_endpoint_filtering_table.py b/keystone/common/sql/migrate_repo/versions/085_add_endpoint_filtering_table.py index 5790bd983..0283bc335 100644 --- a/keystone/common/sql/migrate_repo/versions/085_add_endpoint_filtering_table.py +++ b/keystone/common/sql/migrate_repo/versions/085_add_endpoint_filtering_table.py @@ -12,12 +12,12 @@ import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades def upgrade(migrate_engine): try: - extension_version = migration_helpers.get_db_version( + extension_version = upgrades.get_db_version( extension='endpoint_filter', engine=migrate_engine) except Exception: diff --git a/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py b/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py index f6bba7d9e..e0d6a4ee5 100644 --- a/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py +++ b/keystone/common/sql/migrate_repo/versions/093_migrate_domains_to_projects.py @@ -14,7 +14,7 @@ import json import sqlalchemy as sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades _PROJECT_TABLE_NAME = 'project' @@ -90,7 +90,7 @@ def upgrade(migrate_engine): # the project domain_id to be its parent_id. We re-enable the constraint # in the end of this method. We also remove the domain_id constraint, # while be recreated a FK to the project_id at the end. - migration_helpers.remove_constraints( + upgrades.remove_constraints( list_existing_project_constraints(project_table, domain_table)) # For each domain, create a project acting as a domain. We ignore the @@ -119,7 +119,7 @@ def upgrade(migrate_engine): session.execute(update) session.commit() - migration_helpers.add_constraints( + upgrades.add_constraints( list_new_project_constraints(project_table)) session.close() diff --git a/keystone/common/sql/migration_helpers.py b/keystone/common/sql/upgrades.py similarity index 100% rename from keystone/common/sql/migration_helpers.py rename to keystone/common/sql/upgrades.py diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index 6b187ece0..a6314528c 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -25,7 +25,7 @@ from testtools import matchers from keystone.cmd import cli from keystone.common import dependency -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades import keystone.conf from keystone.i18n import _ from keystone.identity.mapping_backends import mapping as identity_mapping @@ -554,19 +554,19 @@ class CliDBSyncTestCase(unit.BaseTestCase): super(CliDBSyncTestCase, self).setUp() self.config_fixture = self.useFixture(config_fixture.Config(CONF)) self.config_fixture.register_cli_opt(cli.command_opt) - migration_helpers.offline_sync_database_to_version = mock.Mock() - migration_helpers.expand_schema = mock.Mock() - migration_helpers.migrate_data = mock.Mock() - migration_helpers.contract_schema = mock.Mock() + upgrades.offline_sync_database_to_version = mock.Mock() + upgrades.expand_schema = mock.Mock() + upgrades.migrate_data = mock.Mock() + upgrades.contract_schema = mock.Mock() self.command_expand = False self.command_migrate = False self.command_contract = False def _assert_correct_call(self, mocked_function): - for func in [migration_helpers.offline_sync_database_to_version, - migration_helpers.expand_schema, - migration_helpers.migrate_data, - migration_helpers.contract_schema]: + for func in [upgrades.offline_sync_database_to_version, + upgrades.expand_schema, + upgrades.migrate_data, + upgrades.contract_schema]: if func == mocked_function: self.assertTrue(func.called) else: @@ -577,28 +577,28 @@ class CliDBSyncTestCase(unit.BaseTestCase): CONF, 'command', self.FakeConfCommand(self))) cli.DbSync.main() self._assert_correct_call( - migration_helpers.offline_sync_database_to_version) + upgrades.offline_sync_database_to_version) def test_db_sync_expand(self): self.command_expand = True self.useFixture(mockpatch.PatchObject( CONF, 'command', self.FakeConfCommand(self))) cli.DbSync.main() - self._assert_correct_call(migration_helpers.expand_schema) + self._assert_correct_call(upgrades.expand_schema) def test_db_sync_migrate(self): self.command_migrate = True self.useFixture(mockpatch.PatchObject( CONF, 'command', self.FakeConfCommand(self))) cli.DbSync.main() - self._assert_correct_call(migration_helpers.migrate_data) + self._assert_correct_call(upgrades.migrate_data) def test_db_sync_contract(self): self.command_contract = True self.useFixture(mockpatch.PatchObject( CONF, 'command', self.FakeConfCommand(self))) cli.DbSync.main() - self._assert_correct_call(migration_helpers.contract_schema) + self._assert_correct_call(upgrades.contract_schema) class TestMappingPopulate(unit.SQLDriverOverrides, unit.TestCase): diff --git a/keystone/tests/unit/test_sql_banned_operations.py b/keystone/tests/unit/test_sql_banned_operations.py index 244eabd1d..630460b70 100644 --- a/keystone/tests/unit/test_sql_banned_operations.py +++ b/keystone/tests/unit/test_sql_banned_operations.py @@ -27,7 +27,7 @@ from keystone.common.sql import contract_repo from keystone.common.sql import data_migration_repo from keystone.common.sql import expand_repo from keystone.common.sql import migrate_repo -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades class DBOperationNotAllowed(Exception): @@ -138,7 +138,7 @@ class KeystoneMigrationsCheckers(test_migrations.WalkVersionsMixin): @property def INIT_VERSION(self): - return migration_helpers.get_init_version( + return upgrades.get_init_version( abs_path=os.path.abspath(os.path.dirname(self.migrate_file))) @property @@ -159,7 +159,7 @@ class KeystoneMigrationsCheckers(test_migrations.WalkVersionsMixin): def migrate_fully(self, repo_name): abs_path = os.path.abspath(os.path.dirname(repo_name)) - init_version = migration_helpers.get_init_version(abs_path=abs_path) + init_version = upgrades.get_init_version(abs_path=abs_path) schema = versioning_api.ControlledSchema.create( self.migrate_engine, abs_path, init_version) max_version = schema.repository.version().version diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index 5ff90c6bb..e58f9e9a7 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -46,7 +46,7 @@ import sqlalchemy.exc from testtools import matchers from keystone.common import sql -from keystone.common.sql import migration_helpers +from keystone.common.sql import upgrades import keystone.conf from keystone.credential.providers import fernet as credential_fernet from keystone.tests import unit @@ -127,9 +127,9 @@ DATA_MIGRATION_REPO = 'data_migration_repo' CONTRACT_REPO = 'contract_repo' -# Test migration_helpers.get_init_version separately to ensure it works before +# Test upgrades.get_init_version separately to ensure it works before # using in the SqlUpgrade tests. -class MigrationHelpersGetInitVersionTests(unit.TestCase): +class SqlUpgradeGetInitVersionTests(unit.TestCase): @mock.patch.object(repository, 'Repository') def test_get_init_version_no_path(self, repo): migrate_versions = mock.MagicMock() @@ -142,7 +142,7 @@ class MigrationHelpersGetInitVersionTests(unit.TestCase): # an exception. with mock.patch('os.path.isdir', return_value=True): # since 0 is the smallest version expect None - version = migration_helpers.get_init_version() + version = upgrades.get_init_version() self.assertIsNone(version) # check that the default path was used as the first argument to the @@ -165,7 +165,7 @@ class MigrationHelpersGetInitVersionTests(unit.TestCase): path = '/keystone/' + LEGACY_REPO + '/' # since 0 is the smallest version expect None - version = migration_helpers.get_init_version(abs_path=path) + version = upgrades.get_init_version(abs_path=path) self.assertIsNone(version) @mock.patch.object(repository, 'Repository') @@ -182,7 +182,7 @@ class MigrationHelpersGetInitVersionTests(unit.TestCase): with mock.patch('os.path.isdir', return_value=True): path = '/keystone/' + LEGACY_REPO + '/' - version = migration_helpers.get_init_version(abs_path=path) + version = upgrades.get_init_version(abs_path=path) self.assertEqual(initial_version, version) @@ -190,9 +190,9 @@ class MigrationRepository(object): def __init__(self, engine, repo_name): self.repo_name = repo_name - self.repo_path = migration_helpers.find_repo(self.repo_name) + self.repo_path = upgrades.find_repo(self.repo_name) self.min_version = ( - migration_helpers.get_init_version(abs_path=self.repo_path)) + upgrades.get_init_version(abs_path=self.repo_path)) self.schema_ = versioning_api.ControlledSchema.create( engine, self.repo_path, self.min_version) self.max_version = self.schema_.repository.version().version @@ -476,7 +476,7 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): ['id', 'policy_id', 'endpoint_id', 'service_id', 'region_id']) - @mock.patch.object(migration_helpers, 'get_db_version', return_value=1) + @mock.patch.object(upgrades, 'get_db_version', return_value=1) def test_endpoint_policy_already_migrated(self, mock_ep): # By setting the return value to 1, the migration has already been @@ -531,7 +531,7 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): self.assertFalse(sp_table.c.auth_url.nullable) self.assertFalse(sp_table.c.sp_url.nullable) - @mock.patch.object(migration_helpers, 'get_db_version', return_value=8) + @mock.patch.object(upgrades, 'get_db_version', return_value=8) def test_federation_already_migrated(self, mock_federation): # By setting the return value to 8, the migration has already been @@ -580,7 +580,7 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): 'consumer_id', 'expires_at']) - @mock.patch.object(migration_helpers, 'get_db_version', return_value=5) + @mock.patch.object(upgrades, 'get_db_version', return_value=5) def test_oauth1_already_migrated(self, mock_oauth1): # By setting the return value to 5, the migration has already been @@ -605,7 +605,7 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): 'expires_at', 'revoked_at', 'audit_chain_id', 'audit_id']) - @mock.patch.object(migration_helpers, 'get_db_version', return_value=2) + @mock.patch.object(upgrades, 'get_db_version', return_value=2) def test_revoke_already_migrated(self, mock_revoke): # By setting the return value to 2, the migration has already been @@ -652,7 +652,7 @@ class SqlLegacyRepoUpgradeTests(SqlMigrateBase): self.upgrade(85) assert_tables_columns_exist() - @mock.patch.object(migration_helpers, 'get_db_version', return_value=2) + @mock.patch.object(upgrades, 'get_db_version', return_value=2) def test_endpoint_filter_already_migrated(self, mock_endpoint_filter): # By setting the return value to 2, the migration has already been @@ -1613,7 +1613,7 @@ class VersionTests(SqlMigrateBase): self.upgrade() self.assertRaises( db_exception.DbMigrationError, - migration_helpers._sync_common_repo, + upgrades._sync_common_repo, self.repos[LEGACY_REPO].max_version - 1) def test_these_are_not_the_migrations_you_are_looking_for(self):