openstack/nova
Code Issues Proposed changes

Remove **kwargs passing in payload __init__

Browse Source 
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 <sfinucan@redhat.com>
This commit is contained in:
Balazs Gibizer 2017-01-10 16:12:27 +01:00 committed by Stephen Finucane
parent 6e64cb032e
commit 3cd0908814
8 changed files with 128 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
nova/compute/utils.py
View File

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

3
nova/exception_wrapper.py
View File

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

4
nova/notifications/objects/aggregate.py
View File

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

21
nova/notifications/objects/base.py
View File

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

8
nova/notifications/objects/exception.py
View File

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

4
nova/notifications/objects/flavor.py
View File

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

80
nova/notifications/objects/instance.py
View File

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

74
nova/tests/unit/notifications/objects/test_notification.py
View File

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