From 504b466295f3d97307216095f450ff8421796319 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 5 May 2016 13:42:49 +0200 Subject: [PATCH] Remove remotable from OVOs In Cinder we don't have an indirection service to forward object methods like Nova does (Conductor) so we shouldn't be defining all methods as remotable since they are not. If we ever decide to have a Conductor we can easily add them back where appropriate, but for now they are only a hindrance: - It's confusing, because most people don't understand what it actually means in Cinder and they just add it to the methods because they see it in other OVO methods. - Every time we change a remote method's signature or add a new remote method to an object the hash of that object changes and we need to update our test_versions test, which is not necessary. - This makes the linking of VOs with their list counterparts problematic, because everyone will need to know that there is no need to bump the version of a list object whenever they add or modify a remotable method. - We are executing unnecessary code with decorators, even if it's not much, it's still wasted CPU time . This patch removes all appearances of remotable and remotable_class decorators thus making our OVOs consistent with our implementation and removing the need of updating our tests on unrelated OVO changes. Change-Id: I18a0f619b4b108c553d9567b5f21f978466f1670 --- cinder/objects/backup.py | 13 +++------ cinder/objects/base.py | 4 +-- cinder/objects/cgsnapshot.py | 9 ++---- cinder/objects/consistencygroup.py | 7 ++--- cinder/objects/service.py | 17 +++++------- cinder/objects/snapshot.py | 17 +++++------- cinder/objects/volume.py | 11 +++----- cinder/objects/volume_attachment.py | 7 ++--- cinder/objects/volume_type.py | 5 +--- cinder/tests/unit/objects/test_objects.py | 34 +++++++++++------------ 10 files changed, 49 insertions(+), 75 deletions(-) diff --git a/cinder/objects/backup.py b/cinder/objects/backup.py index 7f09800fa2a..9f5778f42ee 100644 --- a/cinder/objects/backup.py +++ b/cinder/objects/backup.py @@ -104,7 +104,6 @@ class Backup(base.CinderPersistentObject, base.CinderObject, backup.obj_reset_changes() return backup - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -114,7 +113,6 @@ class Backup(base.CinderPersistentObject, base.CinderObject, db_backup = db.backup_create(self._context, updates) self._from_db_object(self._context, self, db_backup) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -122,7 +120,6 @@ class Backup(base.CinderPersistentObject, base.CinderObject, self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): db.backup_destroy(self._context, self.id) @@ -141,7 +138,6 @@ class Backup(base.CinderPersistentObject, base.CinderObject, msg = _("Can't parse backup record.") raise exception.InvalidInput(reason=msg) - @base.remotable def encode_record(self, **kwargs): """Serialize backup object, with optional extra info, into a string.""" # We don't want to export extra fields and we want to force lazy @@ -163,7 +159,7 @@ class BackupList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('Backup'), } - @base.remotable_classmethod + @classmethod def get_all(cls, context, filters=None, marker=None, limit=None, offset=None, sort_keys=None, sort_dirs=None): backups = db.backup_get_all(context, filters, marker, limit, offset, @@ -171,13 +167,13 @@ class BackupList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.Backup, backups) - @base.remotable_classmethod + @classmethod def get_all_by_host(cls, context, host): backups = db.backup_get_all_by_host(context, host) return base.obj_make_list(context, cls(context), objects.Backup, backups) - @base.remotable_classmethod + @classmethod def get_all_by_project(cls, context, project_id, filters=None, marker=None, limit=None, offset=None, sort_keys=None, sort_dirs=None): @@ -187,7 +183,7 @@ class BackupList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.Backup, backups) - @base.remotable_classmethod + @classmethod def get_all_by_volume(cls, context, volume_id, filters=None): backups = db.backup_get_all_by_volume(context, volume_id, filters) return base.obj_make_list(context, cls(context), objects.Backup, @@ -208,7 +204,6 @@ class BackupImport(Backup): completed. """ - @base.remotable def create(self): updates = self.cinder_obj_get_changes() diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 8495158f12c..d5fd6c77a20 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -29,8 +29,6 @@ from cinder import objects LOG = logging.getLogger('object') -remotable = base.remotable -remotable_classmethod = base.remotable_classmethod obj_make_list = base.obj_make_list @@ -154,7 +152,7 @@ class CinderObject(base.VersionedObject): def _get_expected_attrs(cls, context): return None - @base.remotable_classmethod + @classmethod def get_by_id(cls, context, id, *args, **kwargs): # To get by id we need to have a model and for the model to # have an id field diff --git a/cinder/objects/cgsnapshot.py b/cinder/objects/cgsnapshot.py index d7b7ba6b7a3..9fa715a0287 100644 --- a/cinder/objects/cgsnapshot.py +++ b/cinder/objects/cgsnapshot.py @@ -68,7 +68,6 @@ class CGSnapshot(base.CinderPersistentObject, base.CinderObject, cgsnapshot.obj_reset_changes() return cgsnapshot - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -101,7 +100,6 @@ class CGSnapshot(base.CinderPersistentObject, base.CinderObject, self.obj_reset_changes(fields=[attrname]) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -114,7 +112,6 @@ class CGSnapshot(base.CinderPersistentObject, base.CinderObject, db.cgsnapshot_update(self._context, self.id, updates) self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): db.cgsnapshot_destroy(self._context, self.id) @@ -128,20 +125,20 @@ class CGSnapshotList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('CGSnapshot') } - @base.remotable_classmethod + @classmethod def get_all(cls, context, filters=None): cgsnapshots = db.cgsnapshot_get_all(context, filters) return base.obj_make_list(context, cls(context), objects.CGSnapshot, cgsnapshots) - @base.remotable_classmethod + @classmethod def get_all_by_project(cls, context, project_id, filters=None): cgsnapshots = db.cgsnapshot_get_all_by_project(context, project_id, filters) return base.obj_make_list(context, cls(context), objects.CGSnapshot, cgsnapshots) - @base.remotable_classmethod + @classmethod def get_all_by_group(cls, context, group_id, filters=None): cgsnapshots = db.cgsnapshot_get_all_by_group(context, group_id, filters) diff --git a/cinder/objects/consistencygroup.py b/cinder/objects/consistencygroup.py index 874de8aa881..7478ce75aaf 100644 --- a/cinder/objects/consistencygroup.py +++ b/cinder/objects/consistencygroup.py @@ -76,7 +76,6 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject, consistencygroup.obj_reset_changes() return consistencygroup - @base.remotable def create(self, cg_snap_id=None, cg_id=None): """Create a consistency group. @@ -122,7 +121,6 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject, self.obj_reset_changes(fields=[attrname]) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -136,7 +134,6 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject, db.consistencygroup_update(self._context, self.id, updates) self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): db.consistencygroup_destroy(self._context, self.id) @@ -152,7 +149,7 @@ class ConsistencyGroupList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('ConsistencyGroup') } - @base.remotable_classmethod + @classmethod def get_all(cls, context, filters=None, marker=None, limit=None, offset=None, sort_keys=None, sort_dirs=None): consistencygroups = db.consistencygroup_get_all( @@ -162,7 +159,7 @@ class ConsistencyGroupList(base.ObjectListBase, base.CinderObject): objects.ConsistencyGroup, consistencygroups) - @base.remotable_classmethod + @classmethod def get_all_by_project(cls, context, project_id, filters=None, marker=None, limit=None, offset=None, sort_keys=None, sort_dirs=None): diff --git a/cinder/objects/service.py b/cinder/objects/service.py index 237d3860955..d64f21be697 100644 --- a/cinder/objects/service.py +++ b/cinder/objects/service.py @@ -68,17 +68,16 @@ class Service(base.CinderPersistentObject, base.CinderObject, service.obj_reset_changes() return service - @base.remotable_classmethod + @classmethod def get_by_host_and_topic(cls, context, host, topic): db_service = db.service_get_by_host_and_topic(context, host, topic) return cls._from_db_object(context, cls(context), db_service) - @base.remotable_classmethod + @classmethod def get_by_args(cls, context, host, binary_key): db_service = db.service_get_by_args(context, host, binary_key) return cls._from_db_object(context, cls(context), db_service) - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -87,14 +86,12 @@ class Service(base.CinderPersistentObject, base.CinderObject, db_service = db.service_create(self._context, updates) self._from_db_object(self._context, self, db_service) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: db.service_update(self._context, self.id, updates) self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): db.service_destroy(self._context, self.id) @@ -120,11 +117,11 @@ class Service(base.CinderPersistentObject, base.CinderObject, return min_ver_str - @base.remotable_classmethod + @classmethod def get_minimum_rpc_version(cls, context, binary): return cls._get_minimum_version('rpc_current_version', context, binary) - @base.remotable_classmethod + @classmethod def get_minimum_obj_version(cls, context, binary): return cls._get_minimum_version('object_current_version', context, binary) @@ -140,20 +137,20 @@ class ServiceList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('Service'), } - @base.remotable_classmethod + @classmethod def get_all(cls, context, filters=None): services = db.service_get_all(context, filters) return base.obj_make_list(context, cls(context), objects.Service, services) - @base.remotable_classmethod + @classmethod def get_all_by_topic(cls, context, topic, disabled=None): services = db.service_get_all_by_topic(context, topic, disabled=disabled) return base.obj_make_list(context, cls(context), objects.Service, services) - @base.remotable_classmethod + @classmethod def get_all_by_binary(cls, context, binary, disabled=None): services = db.service_get_all_by_binary(context, binary, disabled=disabled) diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index 18f93e2b378..ebcfc7dfcc7 100644 --- a/cinder/objects/snapshot.py +++ b/cinder/objects/snapshot.py @@ -139,7 +139,6 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, snapshot.obj_reset_changes() return snapshot - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -156,7 +155,6 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, db_snapshot = db.snapshot_create(self._context, updates) self._from_db_object(self._context, self, db_snapshot) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -179,7 +177,6 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, self.obj_reset_changes() - @base.remotable def destroy(self): db.snapshot_destroy(self._context, self.id) @@ -212,7 +209,7 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, if not md_was_changed: self.obj_reset_changes(['metadata']) - @base.remotable_classmethod + @classmethod def snapshot_data_get_for_project(cls, context, project_id, volume_type_id=None): return db.snapshot_data_get_for_project(context, project_id, @@ -227,7 +224,7 @@ class SnapshotList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('Snapshot'), } - @base.remotable_classmethod + @classmethod def get_all(cls, context, search_opts, marker=None, limit=None, sort_keys=None, sort_dirs=None, offset=None): snapshots = db.snapshot_get_all(context, search_opts, marker, limit, @@ -236,14 +233,14 @@ class SnapshotList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.Snapshot, snapshots, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_by_host(cls, context, host, filters=None): snapshots = db.snapshot_get_by_host(context, host, filters) expected_attrs = Snapshot._get_expected_attrs(context) return base.obj_make_list(context, cls(context), objects.Snapshot, snapshots, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_by_project(cls, context, project_id, search_opts, marker=None, limit=None, sort_keys=None, sort_dirs=None, offset=None): @@ -254,21 +251,21 @@ class SnapshotList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.Snapshot, snapshots, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_for_volume(cls, context, volume_id): snapshots = db.snapshot_get_all_for_volume(context, volume_id) expected_attrs = Snapshot._get_expected_attrs(context) return base.obj_make_list(context, cls(context), objects.Snapshot, snapshots, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_active_by_window(cls, context, begin, end): snapshots = db.snapshot_get_active_by_window(context, begin, end) expected_attrs = Snapshot._get_expected_attrs(context) return base.obj_make_list(context, cls(context), objects.Snapshot, snapshots, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_for_cgsnapshot(cls, context, cgsnapshot_id): snapshots = db.snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id) expected_attrs = Snapshot._get_expected_attrs(context) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index d14d9297cda..168c84ae8c1 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -282,7 +282,6 @@ class Volume(base.CinderPersistentObject, base.CinderObject, volume.obj_reset_changes() return volume - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -299,7 +298,6 @@ class Volume(base.CinderPersistentObject, base.CinderObject, db_volume = db.volume_create(self._context, updates) self._from_db_object(self._context, self, db_volume) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -327,7 +325,6 @@ class Volume(base.CinderPersistentObject, base.CinderObject, db.volume_update(self._context, self.id, updates) self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): db.volume_destroy(self._context, self.id) @@ -449,7 +446,7 @@ class VolumeList(base.ObjectListBase, base.CinderObject): return expected_attrs - @base.remotable_classmethod + @classmethod def get_all(cls, context, marker, limit, sort_keys=None, sort_dirs=None, filters=None, offset=None): volumes = db.volume_get_all(context, marker, limit, @@ -459,21 +456,21 @@ class VolumeList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.Volume, volumes, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_by_host(cls, context, host, filters=None): volumes = db.volume_get_all_by_host(context, host, filters) expected_attrs = cls._get_expected_attrs(context) return base.obj_make_list(context, cls(context), objects.Volume, volumes, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_by_group(cls, context, group_id, filters=None): volumes = db.volume_get_all_by_group(context, group_id, filters) expected_attrs = cls._get_expected_attrs(context) return base.obj_make_list(context, cls(context), objects.Volume, volumes, expected_attrs=expected_attrs) - @base.remotable_classmethod + @classmethod def get_all_by_project(cls, context, project_id, marker, limit, sort_keys=None, sort_dirs=None, filters=None, offset=None): diff --git a/cinder/objects/volume_attachment.py b/cinder/objects/volume_attachment.py index 4c2e0e19e4f..1bf852f0190 100644 --- a/cinder/objects/volume_attachment.py +++ b/cinder/objects/volume_attachment.py @@ -52,7 +52,6 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject, attachment.obj_reset_changes() return attachment - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -68,7 +67,7 @@ class VolumeAttachmentList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('VolumeAttachment'), } - @base.remotable_classmethod + @classmethod def get_all_by_volume_id(cls, context, volume_id): attachments = db.volume_attachment_get_all_by_volume_id(context, volume_id) @@ -77,7 +76,7 @@ class VolumeAttachmentList(base.ObjectListBase, base.CinderObject): objects.VolumeAttachment, attachments) - @base.remotable_classmethod + @classmethod def get_all_by_host(cls, context, volume_id, host): attachments = db.volume_attachment_get_all_by_host(context, volume_id, @@ -85,7 +84,7 @@ class VolumeAttachmentList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.VolumeAttachment, attachments) - @base.remotable_classmethod + @classmethod def get_all_by_instance_uuid(cls, context, volume_id, instance_uuid): attachments = db.volume_attachment_get_all_by_instance_uuid( context, volume_id, instance_uuid) diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index b8f5d4857d1..881e5a89a45 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -72,7 +72,6 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, type.obj_reset_changes() return type - @base.remotable def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', @@ -83,7 +82,6 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, self.description) self._from_db_object(self._context, self, db_volume_type) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() if updates: @@ -91,7 +89,6 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, self.description) self.obj_reset_changes() - @base.remotable def destroy(self): with self.obj_as_admin(): volume_types.destroy(self._context, self.id) @@ -107,7 +104,7 @@ class VolumeTypeList(base.ObjectListBase, base.CinderObject): 'objects': fields.ListOfObjectsField('VolumeType'), } - @base.remotable_classmethod + @classmethod def get_all(cls, context, inactive=0, filters=None, marker=None, limit=None, sort_keys=None, sort_dirs=None, offset=None): types = volume_types.get_all_types(context, inactive, filters, diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 1b44924e791..03b028f4e65 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -23,23 +23,23 @@ from cinder import test # NOTE: The hashes in this list should only be changed if they come with a # corresponding version bump in the affected objects. object_data = { - 'Backup': '1.4-bcd1797dc2f3e17a46571525e9dbec30', - 'BackupImport': '1.4-bcd1797dc2f3e17a46571525e9dbec30', - 'BackupList': '1.0-7350483276ddb74960c8c39b69192eaa', - 'CGSnapshot': '1.0-de2586a31264d7647f40c762dece9d58', - 'CGSnapshotList': '1.0-e8c3f4078cd0ee23487b34d173eec776', - 'ConsistencyGroup': '1.2-de280886bd04d7e3184c1f7c3a7e2074', - 'ConsistencyGroupList': '1.1-73916823b697dfa0c7f02508d87e0f28', - 'Service': '1.3-66c8e1683f58546c54551e9ff0a3b111', - 'ServiceList': '1.1-07d2be494d704784ad2af4d4c91e68e5', - 'Snapshot': '1.1-ac41f2fe2fb0e34127155d1ec6e4c7e0', - 'SnapshotList': '1.0-58441afd20ddf9417b8879aa6de1ee6f', - 'Volume': '1.3-049e3e5dc411b1a4deb7d6ee4f1ad5ef', - 'VolumeList': '1.1-8859f973dc02e9eb4582063a171bd0f1', - 'VolumeAttachment': '1.0-8fc9a9ac6f554fdf2a194d25dbf28a3b', - 'VolumeAttachmentList': '1.0-4ef79b3824e5d1717ebe0d0558ddff96', - 'VolumeType': '1.0-dd980cfd1eef2dcce941a981eb469fc8', - 'VolumeTypeList': '1.1-68a4549e98563caec82a2018638fa69c', + 'Backup': '1.4-c50f7a68bb4c400dd53dd219685b3992', + 'BackupImport': '1.4-c50f7a68bb4c400dd53dd219685b3992', + 'BackupList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'CGSnapshot': '1.0-3212ac2b4c2811b7134fb9ba2c49ff74', + 'CGSnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'ConsistencyGroup': '1.2-ff7638e03ae7a3bb7a43a6c5c4d0c94a', + 'ConsistencyGroupList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'Service': '1.3-d7c1e133791c9d766596a0528fc9a12f', + 'ServiceList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'Snapshot': '1.1-37966f7141646eb29e9ad5298ff2ca8a', + 'SnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'Volume': '1.3-15ff1f42d4e8eb321aa8217dd46aa1e1', + 'VolumeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'VolumeAttachment': '1.0-b30dacf62b2030dd83d8a1603f1064ff', + 'VolumeAttachmentList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'VolumeType': '1.0-6673dd9ce7c27e9c85279afb20833877', + 'VolumeTypeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', }