openstack
/
cinder
Code Issues Proposed changes

Use abstract class for the backup driver interface 

Instead of using NotImplementedError exceptions to define the interface
it's better to use python abc class [1]. The advantage that it fails
faster if a class doesn't implement the needed interface.
See also [2].

[1]: http://legacy.python.org/dev/peps/pep-3119/
[2]: http://dbader.org/blog/abstract-base-classes-in-python

Change-Id: I625a73f70ac5c6ca68ee38a70c98d999454a817e
Partial-Bug: #1346797
This commit is contained in:
Marc Koderer 2014-07-22 13:53:45 +02:00
parent 4b973e90e0
commit 64f6bebc66
6 changed files with 35 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
cinder/backup/driver.py
View File

@ -15,7 +15,10 @@
"""Base class for all backup drivers."""
import abc
from oslo.config import cfg
import six
from cinder.db import base
from cinder import exception
@ -241,6 +244,7 @@ class BackupMetadataAPI(base.Base):
LOG.debug(msg)
@six.add_metaclass(abc.ABCMeta)
class BackupDriver(base.Base):
def __init__(self, context, db_driver=None):
@ -254,17 +258,20 @@ class BackupDriver(base.Base):
def put_metadata(self, volume_id, json_metadata):
self.backup_meta_api.put(volume_id, json_metadata)
@abc.abstractmethod
def backup(self, backup, volume_file, backup_metadata=False):
"""Start a backup of a specified volume."""
raise NotImplementedError()
return
@abc.abstractmethod
def restore(self, backup, volume_id, volume_file):
"""Restore a saved backup."""
raise NotImplementedError()
return
@abc.abstractmethod
def delete(self, backup):
"""Delete a saved backup."""
raise NotImplementedError()
return
def export_record(self, backup):
"""Export backup record.
@ -290,6 +297,10 @@ class BackupDriver(base.Base):
"""
return jsonutils.loads(backup_url.decode("base64"))
@six.add_metaclass(abc.ABCMeta)
class BackupDriverWithVerify(BackupDriver):
@abc.abstractmethod
def verify(self, backup):
"""Verify that the backup exists on the backend.
@ -299,4 +310,4 @@ class BackupDriver(base.Base):
:param backup: backup id of the backup to verify
:raises: InvalidBackup, NotImplementedError
"""
raise NotImplementedError()
return

15
cinder/backup/manager.py
View File

@ -36,6 +36,7 @@ Volume backups can be created, restored, deleted and listed.
from oslo.config import cfg
from oslo import messaging
from cinder.backup import driver
from cinder.backup import rpcapi as backup_rpcapi
from cinder import context
from cinder import exception
@ -560,12 +561,14 @@ class BackupManager(manager.SchedulerDependentManager):
# Verify backup
try:
backup_service.verify(backup_id)
except NotImplementedError:
LOG.warn(_('Backup service %(service)s does not support '
'verify. Backup id %(id)s is not verified. '
'Skipping verify.') % {'service': self.driver_name,
'id': backup_id})
if isinstance(backup_service, driver.BackupDriverWithVerify):
backup_service.verify(backup_id)
else:
LOG.warn(_('Backup service %(service)s does not support '
'verify. Backup id %(id)s is not verified. '
'Skipping verify.') % {'service':
self.driver_name,
'id': backup_id})
except exception.InvalidBackup as err:
with excutils.save_and_reraise_exception():
self.db.backup_update(context, backup_id,

2
cinder/tests/backup/fake_service.py
View File

@ -21,7 +21,7 @@ LOG = logging.getLogger(__name__)
class FakeBackupService(BackupDriver):
def __init__(self, context, db_driver=None):
super(FakeBackupService, self).__init__(db_driver)
super(FakeBackupService, self).__init__(context, db_driver)
def backup(self, backup, volume_file):
pass

4
cinder/tests/backup/fake_service_with_verify.py
View File

@ -13,13 +13,15 @@
# License for the specific language governing permissions and limitations
# under the License.
from cinder.backup import driver
from cinder.openstack.common import log as logging
from cinder.tests.backup import fake_service
LOG = logging.getLogger(__name__)
class FakeBackupServiceWithVerify(fake_service.FakeBackupService):
class FakeBackupServiceWithVerify(driver.BackupDriverWithVerify,
fake_service.FakeBackupService):
def verify(self, backup):
pass

17
cinder/tests/test_backup.py
View File

@ -470,18 +470,11 @@ class BackupTestCase(BaseBackupTest):
export = self._create_exported_record_entry(vol_size=vol_size)
imported_record = self._create_export_record_db_entry()
backup_hosts = []
backup_driver = self.backup_mgr.service.get_backup_driver(self.ctxt)
_mock_backup_verify_class = ('%s.%s.%s' %
(backup_driver.__module__,
backup_driver.__class__.__name__,
'verify'))
with mock.patch(_mock_backup_verify_class) as _mock_record_verify:
_mock_record_verify.side_effect = NotImplementedError()
self.backup_mgr.import_record(self.ctxt,
imported_record,
export['backup_service'],
export['backup_url'],
backup_hosts)
self.backup_mgr.import_record(self.ctxt,
imported_record,
export['backup_service'],
export['backup_url'],
backup_hosts)
backup = db.backup_get(self.ctxt, imported_record)
self.assertEqual(backup['status'], 'available')
self.assertEqual(backup['size'], vol_size)

21
cinder/tests/test_backup_driver_base.py
View File

@ -24,7 +24,7 @@ from cinder import db
from cinder import exception
from cinder.openstack.common import jsonutils
from cinder import test
from cinder.tests.backup import fake_service
_backup_db_fields = ['id', 'user_id', 'project_id',
'volume_id', 'host', 'availability_zone',
@ -54,20 +54,7 @@ class BackupBaseDriverTestCase(test.TestCase):
self._create_backup_db_entry(self.backup_id, self.volume_id, 1)
self._create_volume_db_entry(self.volume_id, 1)
self.backup = db.backup_get(self.ctxt, self.backup_id)
self.driver = driver.BackupDriver(self.ctxt)
def test_backup(self):
self.assertRaises(NotImplementedError,
self.driver.backup, self.backup, self.volume_id)
def test_restore(self):
self.assertRaises(NotImplementedError,
self.driver.restore, self.backup, self.volume_id,
None)
def test_delete(self):
self.assertRaises(NotImplementedError,
self.driver.delete, self.backup)
self.driver = fake_service.FakeBackupService(self.ctxt)
def test_get_metadata(self):
json_metadata = self.driver.get_metadata(self.volume_id)
@ -98,10 +85,6 @@ class BackupBaseDriverTestCase(test.TestCase):
self.assertTrue(key in imported_backup)
self.assertEqual(imported_backup[key], self.backup[key])
def test_verify(self):
self.assertRaises(NotImplementedError,
self.driver.verify, self.backup)
class BackupMetadataAPITestCase(test.TestCase):