openstack/deb-python-oslo.db
Code Issues Proposed changes

exception: make message mandatory in DbMigrationError and deprecates it

Browse Source 
An exception message with None as an error message is not really useful. Since
DbMigrationError is also wrongly written with a lower case b whereas everything
else has a major B, let's create a new correct class and use it.

debtcollector is used to deprecate the old one, and inheritance makes sure the
current code works.

Change-Id: Id9a477aff38afd5b664b75a549ad561f4c24b6f0
This commit is contained in:
Julien Danjou
2016-02-04 15:11:45 +01:00
parent 1579c7ce15
commit e0db469371
7 changed files with 38 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
oslo_db/exception.py
View File

@@ -171,6 +171,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.

18
oslo_db/sqlalchemy/migration.py
View File

@@ -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)

4
oslo_db/sqlalchemy/migration_cli/manager.py
View File

@@ -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 = []

2
oslo_db/sqlalchemy/test_migrations.py
View File

@@ -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

4
oslo_db/tests/sqlalchemy/test_migrate_cli.py
View File

@@ -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

10
oslo_db/tests/sqlalchemy/test_migration_common.py
View File

@@ -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):

8
oslo_db/tests/sqlalchemy/test_migrations.py
View File

@@ -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):