From 3cd09088142a0ae48fbc3de562edf7baa61cc174 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Tue, 10 Jan 2017 16:12:27 +0100 Subject: [PATCH] Remove **kwargs passing in payload __init__ The **kwargs was used in the payload class ctor to pass the payload fields initialization up to the ovo base class. This is unnecessary and confusing. This patch replaces the **kwargs passing with direct field initialization to make the payload construction more explicit. Change-Id: I7770f6550fa40c1d9b0417efa57f58fa9c998d45 Implements: bp versioned-notification-transformation-pike Co-Authored-By: Stephen Finucane --- nova/compute/utils.py | 8 +- nova/exception_wrapper.py | 3 +- nova/notifications/objects/aggregate.py | 4 +- nova/notifications/objects/base.py | 21 +++-- nova/notifications/objects/exception.py | 8 ++ nova/notifications/objects/flavor.py | 4 +- nova/notifications/objects/instance.py | 80 ++++++++++++------- .../objects/test_notification.py | 74 ++++++++++------- 8 files changed, 128 insertions(+), 74 deletions(-) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 823949be9e57..8fde853e4ee1 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -357,7 +357,7 @@ def notify_about_instance_action(context, instance, host, action, phase=None, context=context, priority=priority, publisher=notification_base.NotificationPublisher( - context=context, host=host, binary=binary), + host=host, binary=binary), event_type=notification_base.EventType( object='instance', action=action, @@ -387,7 +387,7 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, context=context, priority=priority, publisher=notification_base.NotificationPublisher( - context=context, host=host, binary=binary), + host=host, binary=binary), event_type=notification_base.EventType( object='instance', action=action, @@ -421,7 +421,7 @@ def notify_about_volume_swap(context, instance, host, action, phase, context=context, priority=priority, publisher=notification_base.NotificationPublisher( - context=context, host=host, binary='nova-compute'), + host=host, binary='nova-compute'), event_type=notification_base.EventType( object='instance', action=action, phase=phase), payload=payload).emit(context) @@ -463,7 +463,7 @@ def notify_about_aggregate_action(context, aggregate, action, phase): notification = aggregate_notification.AggregateNotification( priority=fields.NotificationPriority.INFO, publisher=notification_base.NotificationPublisher( - context=context, host=CONF.host, binary='nova-api'), + host=CONF.host, binary='nova-api'), event_type=notification_base.EventType( object='aggregate', action=action, diff --git a/nova/exception_wrapper.py b/nova/exception_wrapper.py index 7ea70c09c2c1..9009b80958f8 100644 --- a/nova/exception_wrapper.py +++ b/nova/exception_wrapper.py @@ -36,8 +36,7 @@ def _emit_exception_notification(notifier, context, ex, function_name, args, @rpc.if_notifications_enabled def _emit_versioned_exception_notification(context, ex, binary): versioned_exception_payload = exception.ExceptionPayload.from_exception(ex) - publisher = base.NotificationPublisher(context=context, host=CONF.host, - binary=binary) + publisher = base.NotificationPublisher(host=CONF.host, binary=binary) event_type = base.EventType( object='compute', action=fields.NotificationAction.EXCEPTION) diff --git a/nova/notifications/objects/aggregate.py b/nova/notifications/objects/aggregate.py index 88e617db553c..d94516090e90 100644 --- a/nova/notifications/objects/aggregate.py +++ b/nova/notifications/objects/aggregate.py @@ -37,8 +37,8 @@ class AggregatePayload(base.NotificationPayloadBase): 'metadata': fields.DictOfStringsField(nullable=True), } - def __init__(self, aggregate, **kwargs): - super(AggregatePayload, self).__init__(**kwargs) + def __init__(self, aggregate): + super(AggregatePayload, self).__init__() self.populate_schema(aggregate=aggregate) diff --git a/nova/notifications/objects/base.py b/nova/notifications/objects/base.py index c55dec5d7840..948824bff3c9 100644 --- a/nova/notifications/objects/base.py +++ b/nova/notifications/objects/base.py @@ -56,10 +56,16 @@ class EventType(NotificationObject): 'phase': fields.NotificationPhaseField(nullable=True), } + def __init__(self, object, action, phase=None): + super(EventType, self).__init__() + self.object = object + self.action = action + self.phase = phase + def to_notification_event_type_field(self): """Serialize the object to the wire format.""" s = '%s.%s' % (self.object, self.action) - if self.obj_attr_is_set('phase'): + if self.phase: s += '.%s' % self.phase return s @@ -89,8 +95,8 @@ class NotificationPayloadBase(NotificationObject): # Version 1.0: Initial version VERSION = '1.0' - def __init__(self, **kwargs): - super(NotificationPayloadBase, self).__init__(**kwargs) + def __init__(self): + super(NotificationPayloadBase, self).__init__() self.populated = not self.SCHEMA @rpc.if_notifications_enabled @@ -126,7 +132,7 @@ class NotificationPayloadBase(NotificationObject): # the schema population will create changed fields but we don't need # this information in the notification - self.obj_reset_changes(recursive=False) + self.obj_reset_changes(recursive=True) @base.NovaObjectRegistry.register_notification @@ -139,6 +145,11 @@ class NotificationPublisher(NotificationObject): 'binary': fields.StringField(nullable=False), } + def __init__(self, host, binary): + super(NotificationPublisher, self).__init__() + self.host = host + self.binary = binary + @classmethod def from_service_obj(cls, service): return cls(host=service.host, binary=service.binary) @@ -172,7 +183,7 @@ class NotificationBase(NotificationObject): # Note(gibi): notification payload will be a newly populated object # therefore every field of it will look changed so this does not carry # any extra information so we drop this from the payload. - self.payload.obj_reset_changes(recursive=False) + self.payload.obj_reset_changes(recursive=True) self._emit(context, event_type= diff --git a/nova/notifications/objects/exception.py b/nova/notifications/objects/exception.py index b769c8b929f2..e0e5016f47d8 100644 --- a/nova/notifications/objects/exception.py +++ b/nova/notifications/objects/exception.py @@ -29,6 +29,14 @@ class ExceptionPayload(base.NotificationPayloadBase): 'exception_message': fields.StringField() } + def __init__(self, module_name, function_name, exception, + exception_message): + super(ExceptionPayload, self).__init__() + self.module_name = module_name + self.function_name = function_name + self.exception = exception + self.exception_message = exception_message + @classmethod def from_exception(cls, fault): trace = inspect.trace()[-1] diff --git a/nova/notifications/objects/flavor.py b/nova/notifications/objects/flavor.py index 03f95cb9eb3f..91e92acbaefa 100644 --- a/nova/notifications/objects/flavor.py +++ b/nova/notifications/objects/flavor.py @@ -73,8 +73,8 @@ class FlavorPayload(base.NotificationPayloadBase): 'projects': fields.ListOfStringsField(nullable=True), } - def __init__(self, flavor, **kwargs): - super(FlavorPayload, self).__init__(**kwargs) + def __init__(self, flavor): + super(FlavorPayload, self).__init__() if 'projects' not in flavor: # NOTE(danms): If projects is not loaded in the flavor, # don't attempt to load it. If we're in a child cell then diff --git a/nova/notifications/objects/instance.py b/nova/notifications/objects/instance.py index dc829a0739bf..fdf759cd5d4b 100644 --- a/nova/notifications/objects/instance.py +++ b/nova/notifications/objects/instance.py @@ -92,21 +92,14 @@ class InstancePayload(base.NotificationPayloadBase): 'auto_disk_config': fields.DiskConfigField() } - def __init__(self, instance, **kwargs): - super(InstancePayload, self).__init__(**kwargs) - + def __init__(self, instance): + super(InstancePayload, self).__init__() # Note(gibi): ugly but needed to avoid cyclic import from nova.compute import utils - network_info = utils.get_nw_info_for_instance(instance) - ips = IpPayload.from_network_info(network_info) - - flavor = flavor_payload.FlavorPayload(flavor=instance.flavor) - - super(InstancePayload, self).__init__( - ip_addresses=ips, - flavor=flavor, - **kwargs) + self.ip_addresses = IpPayload.from_network_info( + utils.get_nw_info_for_instance(instance)) + self.flavor = flavor_payload.FlavorPayload(flavor=instance.flavor) self.populate_schema(instance=instance) @@ -122,11 +115,9 @@ class InstanceActionPayload(InstancePayload): 'fault': fields.ObjectField('ExceptionPayload', nullable=True), } - def __init__(self, instance, fault, **kwargs): - super(InstanceActionPayload, self).__init__( - instance=instance, - fault=fault, - **kwargs) + def __init__(self, instance, fault): + super(InstanceActionPayload, self).__init__(instance=instance) + self.fault = fault @nova_base.NovaObjectRegistry.register_notification @@ -141,8 +132,8 @@ class InstanceActionVolumePayload(InstanceActionPayload): def __init__(self, instance, fault, volume_id): super(InstanceActionVolumePayload, self).__init__( instance=instance, - fault=fault, - volume_id=volume_id) + fault=fault) + self.volume_id = volume_id @nova_base.NovaObjectRegistry.register_notification @@ -160,9 +151,9 @@ class InstanceActionVolumeSwapPayload(InstanceActionPayload): def __init__(self, instance, fault, old_volume_id, new_volume_id): super(InstanceActionVolumeSwapPayload, self).__init__( instance=instance, - fault=fault, - old_volume_id=old_volume_id, - new_volume_id=new_volume_id) + fault=fault) + self.old_volume_id = old_volume_id + self.new_volume_id = new_volume_id @nova_base.NovaObjectRegistry.register_notification @@ -182,15 +173,13 @@ class InstanceUpdatePayload(InstancePayload): def __init__(self, instance, state_update, audit_period, bandwidth, old_display_name): - tags = [instance_tag.tag for instance_tag in instance.tags.objects] - - super(InstanceUpdatePayload, self).__init__( - instance=instance, - state_update=state_update, - audit_period=audit_period, - bandwidth=bandwidth, - old_display_name=old_display_name, - tags=tags) + super(InstanceUpdatePayload, self).__init__(instance=instance) + self.state_update = state_update + self.audit_period = audit_period + self.bandwidth = bandwidth + self.old_display_name = old_display_name + self.tags = [instance_tag.tag + for instance_tag in instance.tags.objects] @nova_base.NovaObjectRegistry.register_notification @@ -207,6 +196,17 @@ class IpPayload(base.NotificationPayloadBase): 'device_name': fields.StringField(nullable=True) } + def __init__(self, label, mac, meta, port_uuid, version, address, + device_name): + super(IpPayload, self).__init__() + self.label = label + self.mac = mac + self.meta = meta + self.port_uuid = port_uuid + self.version = version + self.address = address + self.device_name = device_name + @classmethod def from_network_info(cls, network_info): """Returns a list of IpPayload object based on the passed @@ -237,6 +237,12 @@ class BandwidthPayload(base.NotificationPayloadBase): 'out_bytes': fields.IntegerField(), } + def __init__(self, network_name, in_bytes, out_bytes): + super(BandwidthPayload, self).__init__() + self.network_name = network_name + self.in_bytes = in_bytes + self.out_bytes = out_bytes + @nova_base.NovaObjectRegistry.register_notification class AuditPeriodPayload(base.NotificationPayloadBase): @@ -247,6 +253,11 @@ class AuditPeriodPayload(base.NotificationPayloadBase): 'audit_period_ending': fields.DateTimeField(), } + def __init__(self, audit_period_beginning, audit_period_ending): + super(AuditPeriodPayload, self).__init__() + self.audit_period_beginning = audit_period_beginning + self.audit_period_ending = audit_period_ending + @nova_base.NovaObjectRegistry.register_notification class InstanceStateUpdatePayload(base.NotificationPayloadBase): @@ -259,6 +270,13 @@ class InstanceStateUpdatePayload(base.NotificationPayloadBase): 'new_task_state': fields.StringField(nullable=True), } + def __init__(self, old_state, state, old_task_state, new_task_state): + super(InstanceStateUpdatePayload, self).__init__() + self.old_state = old_state + self.state = state + self.old_task_state = old_task_state + self.new_task_state = new_task_state + @base.notification_sample('instance-delete-start.json') @base.notification_sample('instance-delete-end.json') diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index d0e815f5e83e..52a6ff3ff511 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -49,6 +49,12 @@ class TestNotificationBase(test.NoDBTestCase): action='obj_load_attr', reason='attribute %s not lazy-loadable' % attrname) + def __init__(self, not_important_field): + super(TestNotificationBase.TestObject, self).__init__() + # field1 and field_2 simulates that some fields are initialized + # outside of the object's ctor + self.not_important_field = not_important_field + @base.NovaObjectRegistry.register_if(False) class TestNotificationPayload(notification.NotificationPayloadBase): VERSION = '1.0' @@ -67,9 +73,11 @@ class TestNotificationBase(test.NoDBTestCase): 'lazy_field': fields.IntegerField() # filled by the schema } - def populate_schema(self, source_field): + def __init__(self, extra_field, source_field): super(TestNotificationBase.TestNotificationPayload, - self).populate_schema(source_field=source_field) + self).__init__() + self.extra_field = extra_field + self.populate_schema(source_field=source_field) @base.NovaObjectRegistry.register_if(False) class TestNotificationPayloadEmptySchema( @@ -80,6 +88,11 @@ class TestNotificationBase(test.NoDBTestCase): 'extra_field': fields.StringField(), # filled by ctor } + def __init__(self, extra_field): + super(TestNotificationBase.TestNotificationPayloadEmptySchema, + self).__init__() + self.extra_field = extra_field + @notification.notification_sample('test-update-1.json') @notification.notification_sample('test-update-2.json') @base.NovaObjectRegistry.register_if(False) @@ -118,7 +131,7 @@ class TestNotificationBase(test.NoDBTestCase): 'nova_object.data': { 'extra_field': 'test string', 'field_1': 'test1', - 'field_2': 42, + 'field_2': 15, 'lazy_field': 42}, 'nova_object.version': '1.0', 'nova_object.namespace': 'nova'} @@ -132,13 +145,12 @@ class TestNotificationBase(test.NoDBTestCase): mock_db_service_update.return_value = self.fake_service self.service_obj.save() - self.my_obj = self.TestObject(field_1='test1', - field_2=42, - not_important_field=13) + self.my_obj = self.TestObject(not_important_field=13) + self.my_obj.field_1 = 'test1' + self.my_obj.field_2 = 15 self.payload = self.TestNotificationPayload( - extra_field='test string') - self.payload.populate_schema(source_field=self.my_obj) + extra_field='test string', source_field=self.my_obj) self.notification = self.TestNotification( event_type=notification.EventType( @@ -224,8 +236,10 @@ class TestNotificationBase(test.NoDBTestCase): @mock.patch('nova.rpc.NOTIFIER') def test_not_possible_to_emit_if_not_populated(self, mock_notifier): - non_populated_payload = self.TestNotificationPayload( - extra_field='test string') + payload = self.TestNotificationPayload( + extra_field='test string', source_field=self.my_obj) + payload.populated = False + noti = self.TestNotification( event_type=notification.EventType( object='test_object', @@ -233,37 +247,41 @@ class TestNotificationBase(test.NoDBTestCase): publisher=notification.NotificationPublisher.from_service_obj( self.service_obj), priority=fields.NotificationPriority.INFO, - payload=non_populated_payload) + payload=payload) mock_context = mock.Mock() self.assertRaises(AssertionError, noti.emit, mock_context) self.assertFalse(mock_notifier.called) def test_lazy_load_source_field(self): - my_obj = self.TestObject(field_1='test1', - field_2=42, - not_important_field=13) - payload = self.TestNotificationPayload(extra_field='test string') + my_obj = self.TestObject(not_important_field=13) + my_obj.field_1 = 'test1' + my_obj.field_2 = 15 - payload.populate_schema(my_obj) + payload = self.TestNotificationPayload(extra_field='test string', + source_field=my_obj) self.assertEqual(42, payload.lazy_field) def test_uninited_source_field_defaulted_to_none(self): - my_obj = self.TestObject(field_2=42, - not_important_field=13) - payload = self.TestNotificationPayload(extra_field='test string') + my_obj = self.TestObject(not_important_field=13) + # intentionally not initializing field_1 to simulate an uninited but + # nullable field + my_obj.field_2 = 15 - payload.populate_schema(my_obj) + payload = self.TestNotificationPayload(extra_field='test string', + source_field=my_obj) self.assertIsNone(payload.field_1) def test_uninited_source_field_not_nullable_payload_field_fails(self): - my_obj = self.TestObject(field_1='test1', - not_important_field=13) - payload = self.TestNotificationPayload(extra_field='test string') + my_obj = self.TestObject(not_important_field=13) + # intentionally not initializing field_2 to simulate an uninited no + # nullable field + my_obj.field_1 = 'test1' - self.assertRaises(ValueError, payload.populate_schema, my_obj) + self.assertRaises(ValueError, self.TestNotificationPayload, + extra_field='test string', source_field=my_obj) @mock.patch('nova.rpc.NOTIFIER') def test_empty_schema(self, mock_notifier): @@ -304,8 +322,8 @@ class TestNotificationBase(test.NoDBTestCase): mock_notifier.is_enabled.return_value = False payload = self.TestNotificationPayload( - extra_field='test string') - self.payload.populate_schema(source_field=self.my_obj) + extra_field='test string', + source_field=self.my_obj) noti = self.TestNotification( event_type=notification.EventType( object='test_object', @@ -328,8 +346,8 @@ class TestNotificationBase(test.NoDBTestCase): self.flags(notification_format='unversioned', group='notifications') payload = self.TestNotificationPayload( - extra_field='test string') - self.payload.populate_schema(source_field=self.my_obj) + extra_field='test string', + source_field=self.my_obj) noti = self.TestNotification( event_type=notification.EventType( object='test_object',