Merge "exception: make message mandatory in DbMigrationError and deprecates it"

7 files changed, 38 insertions, 22 deletions

diff --git a/oslo_db/exception.py b/oslo_db/exception.py
index 9c0c6fb..16dec7f 100644
--- a/oslo_db/exception.py
+++ b/oslo_db/exception.py
@@ -198,6 +198,20 @@ class DbMigrationError(DBError):
         super(DbMigrationError, self).__init__(message)
 
 
+class DBMigrationError(DbMigrationError):
+
+    """Wrapped migration specific exception.
+
+    Raised when migrations couldn't be completed successfully.
+    """
+    def __init__(self, message):
+        super(DBMigrationError, self).__init__(message)
+
+
+debtcollector.removals.removed_class(DbMigrationError,
+                                     replacement=DBMigrationError)
+
+
 class DBConnectionError(DBError):
 
     """Wrapped connection specific exception.
diff --git a/oslo_db/sqlalchemy/migration.py b/oslo_db/sqlalchemy/migration.py
index c85fd22..cbc8b28 100644
--- a/oslo_db/sqlalchemy/migration.py
+++ b/oslo_db/sqlalchemy/migration.py
@@ -68,8 +68,7 @@ def db_sync(engine, abs_path, version=None, init_version=0, sanity_check=True):
         try:
             version = int(version)
         except ValueError:
-            raise exception.DbMigrationError(
-                message=_("version should be an integer"))
+            raise exception.DBMigrationError(_("version should be an integer"))
 
     current_version = db_version(engine, abs_path, init_version)
     repository = _find_migrate_repo(abs_path)
@@ -132,11 +131,10 @@ def db_version(engine, abs_path, init_version):
             db_version_control(engine, abs_path, version=init_version)
             return versioning_api.db_version(engine, repository)
         else:
-            raise exception.DbMigrationError(
-                message=_(
-                    "The database is not under version control, but has "
-                    "tables. Please stamp the current version of the schema "
-                    "manually."))
+            raise exception.DBMigrationError(
+                _("The database is not under version control, but has "
+                  "tables. Please stamp the current version of the schema "
+                  "manually."))
 
 
 def db_version_control(engine, abs_path, version=None):
@@ -154,9 +152,9 @@ def db_version_control(engine, abs_path, version=None):
     try:
         versioning_api.version_control(engine, repository, version)
     except versioning_exceptions.InvalidVersionError as ex:
-        raise exception.DbMigrationError("Invalid version : %s" % ex)
+        raise exception.DBMigrationError("Invalid version : %s" % ex)
     except versioning_exceptions.DatabaseAlreadyControlledError:
-        raise exception.DbMigrationError("Database is already controlled.")
+        raise exception.DBMigrationError("Database is already controlled.")
 
     return version
 
@@ -167,5 +165,5 @@ def _find_migrate_repo(abs_path):
     :param abs_path: Absolute path to migrate repository
     """
     if not os.path.exists(abs_path):
-        raise exception.DbMigrationError("Path %s not found" % abs_path)
+        raise exception.DBMigrationError("Path %s not found" % abs_path)
     return Repository(abs_path)
diff --git a/oslo_db/sqlalchemy/migration_cli/manager.py b/oslo_db/sqlalchemy/migration_cli/manager.py
index d335fa5..4268331 100644
--- a/oslo_db/sqlalchemy/migration_cli/manager.py
+++ b/oslo_db/sqlalchemy/migration_cli/manager.py
@@ -57,7 +57,7 @@ class MigrationManager(object):
         # revision=None is a special case meaning latest revision.
         rev_in_plugins = [p.has_revision(revision) for p in self._plugins]
         if not any(rev_in_plugins) and revision is not None:
-            raise exception.DbMigrationError('Revision does not exist')
+            raise exception.DBMigrationError('Revision does not exist')
 
         results = []
         for plugin, has_revision in zip(self._plugins, rev_in_plugins):
@@ -75,7 +75,7 @@ class MigrationManager(object):
         # revision=None is a special case meaning initial revision.
         rev_in_plugins = [p.has_revision(revision) for p in self._plugins]
         if not any(rev_in_plugins) and revision is not None:
-            raise exception.DbMigrationError('Revision does not exist')
+            raise exception.DBMigrationError('Revision does not exist')
 
         # downgrading should be performed in reversed order
         results = []
diff --git a/oslo_db/sqlalchemy/test_migrations.py b/oslo_db/sqlalchemy/test_migrations.py
index df53b85..524a339 100644
--- a/oslo_db/sqlalchemy/test_migrations.py
+++ b/oslo_db/sqlalchemy/test_migrations.py
@@ -268,7 +268,7 @@ class WalkVersionsMixin(object):
                 check = getattr(self, "_check_%03d" % version, None)
                 if check:
                     check(self.migrate_engine, data)
-        except exc.DbMigrationError:
+        except exc.DBMigrationError:
             msg = _LE("Failed to migrate to version %(ver)s on engine %(eng)s")
             LOG.error(msg, {"ver": version, "eng": self.migrate_engine})
             raise
diff --git a/oslo_db/tests/sqlalchemy/test_migrate_cli.py b/oslo_db/tests/sqlalchemy/test_migrate_cli.py
index 8d2ae8d..7ee9450 100644
--- a/oslo_db/tests/sqlalchemy/test_migrate_cli.py
+++ b/oslo_db/tests/sqlalchemy/test_migrate_cli.py
@@ -314,7 +314,7 @@ class TestMigrationMultipleExtensions(test_base.BaseTestCase):
         self.second_ext.obj.has_revision.return_value = False
 
         # upgrade to a specific non-existent revision should fail
-        self.assertRaises(exception.DbMigrationError,
+        self.assertRaises(exception.DBMigrationError,
                           self.migration_manager.upgrade, 100)
 
         # upgrade to the "head" should succeed
@@ -332,7 +332,7 @@ class TestMigrationMultipleExtensions(test_base.BaseTestCase):
         self.second_ext.obj.has_revision.return_value = False
 
         # upgrade to a specific non-existent revision should fail
-        self.assertRaises(exception.DbMigrationError,
+        self.assertRaises(exception.DBMigrationError,
                           self.migration_manager.downgrade, 100)
 
         # downgrade to the "base" should succeed
diff --git a/oslo_db/tests/sqlalchemy/test_migration_common.py b/oslo_db/tests/sqlalchemy/test_migration_common.py
index f6f5c95..5dfcdc9 100644
--- a/oslo_db/tests/sqlalchemy/test_migration_common.py
+++ b/oslo_db/tests/sqlalchemy/test_migration_common.py
@@ -56,7 +56,7 @@ class TestMigrationCommon(test_base.DbTestCase):
 
     def test_find_migrate_repo_path_not_found(self):
         self.assertRaises(
-            db_exception.DbMigrationError,
+            db_exception.DBMigrationError,
             migration._find_migrate_repo,
             "/foo/bar/",
         )
@@ -93,7 +93,7 @@ class TestMigrationCommon(test_base.DbTestCase):
         mock_find_repo.return_value = self.return_value
         mock_version_control.side_effect = (migrate_exception.
                                             DatabaseAlreadyControlledError)
-        self.assertRaises(db_exception.DbMigrationError,
+        self.assertRaises(db_exception.DBMigrationError,
                           migration.db_version_control, self.engine,
                           self.path, self.test_version - 1)
 
@@ -104,7 +104,7 @@ class TestMigrationCommon(test_base.DbTestCase):
         mock_find_repo.return_value = self.return_value
         mock_version_control.side_effect = (migrate_exception.
                                             InvalidVersionError)
-        self.assertRaises(db_exception.DbMigrationError,
+        self.assertRaises(db_exception.DBMigrationError,
                           migration.db_version_control, self.engine,
                           self.path, self.test_version + 1)
 
@@ -135,7 +135,7 @@ class TestMigrationCommon(test_base.DbTestCase):
             mock_meta.return_value = my_meta
 
             self.assertRaises(
-                db_exception.DbMigrationError, migration.db_version,
+                db_exception.DBMigrationError, migration.db_version,
                 self.engine, self.path, self.init_version)
 
     @mock.patch.object(versioning_api, 'version_control')
@@ -153,7 +153,7 @@ class TestMigrationCommon(test_base.DbTestCase):
                                             self.init_version)
 
     def test_db_sync_wrong_version(self):
-        self.assertRaises(db_exception.DbMigrationError,
+        self.assertRaises(db_exception.DBMigrationError,
                           migration.db_sync, self.engine, self.path, 'foo')
 
     def test_db_sync_upgrade(self):
diff --git a/oslo_db/tests/sqlalchemy/test_migrations.py b/oslo_db/tests/sqlalchemy/test_migrations.py
index 65ad6f8..1ed5510 100644
--- a/oslo_db/tests/sqlalchemy/test_migrations.py
+++ b/oslo_db/tests/sqlalchemy/test_migrations.py
@@ -47,6 +47,10 @@ class TestWalkVersions(test.BaseTestCase, migrate.WalkVersionsMixin):
         self.migration_api.db_version.assert_called_with(
             self.engine, self.REPOSITORY)
 
+    @staticmethod
+    def _fake_upgrade_boom(*args, **kwargs):
+        raise exc.DBMigrationError("boom")
+
     def test_migrate_up_fail(self):
         version = 141
         self.migration_api.db_version.return_value = version
@@ -56,9 +60,9 @@ class TestWalkVersions(test.BaseTestCase, migrate.WalkVersionsMixin):
 
         with mock.patch.object(self.migration_api,
                                'upgrade',
-                               side_effect=exc.DbMigrationError):
+                               side_effect=self._fake_upgrade_boom):
             log = self.useFixture(fixtures.FakeLogger())
-            self.assertRaises(exc.DbMigrationError, self.migrate_up, version)
+            self.assertRaises(exc.DBMigrationError, self.migrate_up, version)
             self.assertEqual(expected_output, log.output)
 
     def test_migrate_up_with_data(self):