From 7a4136df187fc6d2f5813580e8868e591007c2e8 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Mon, 2 Mar 2015 17:19:44 -0500 Subject: [PATCH] Better round trip for RequestContext<->Dict conversion Conversion of nova.context.RequestContext to and from a dictionary was not working properly. There is duplicate information as well in the serialized dict. As a first step i've removed request_id as it dups the one in oslo_context.RequestContext. However changing project_id and user_id to a @property is challenging at this late juncture. So leaving them as-is with a FIXME. The other issue related to the roundtrip is the stack traces that show up in screen-n-cpu log as mentioned in the bug referenced below. To avoid that added some defensive code ('has_attr') for now and a FIXME. Partial-Bug: #1427665 Change-Id: Ia47d4909d2656d6fc4c1179659b8098bba3235d3 --- nova/context.py | 59 +++++++++++++++++++-------------- nova/tests/unit/test_context.py | 58 ++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/nova/context.py b/nova/context.py index 4610d4856514..a1963f81e7b1 100644 --- a/nova/context.py +++ b/nova/context.py @@ -67,7 +67,8 @@ class RequestContext(context.RequestContext): """ - def __init__(self, user_id, project_id, is_admin=None, read_deleted="no", + def __init__(self, user_id=None, project_id=None, + is_admin=None, read_deleted="no", roles=None, remote_address=None, timestamp=None, request_id=None, auth_token=None, overwrite=True, quota_class=None, user_name=None, project_name=None, @@ -86,16 +87,22 @@ class RequestContext(context.RequestContext): :param kwargs: Extra arguments that might be present, but we ignore because they possibly came in from older rpc messages. """ - super(RequestContext, self).__init__(auth_token=auth_token, - user=user_id, - tenant=project_id, - is_admin=is_admin, - request_id=request_id, - overwrite=overwrite) + user = kwargs.pop('user', None) + tenant = kwargs.pop('tenant', None) + super(RequestContext, self).__init__( + auth_token=auth_token, + user=user_id or user, + tenant=project_id or tenant, + is_admin=is_admin, + request_id=request_id, + overwrite=overwrite) if kwargs: LOG.warning(_LW('Arguments dropped when creating context: %s') % str(kwargs)) + # FIXME(dims): user_id and project_id duplicate information that is + # already present in the oslo_context's RequestContext. We need to + # get rid of them. self.user_id = user_id self.project_id = project_id self.roles = roles or [] @@ -106,9 +113,6 @@ class RequestContext(context.RequestContext): if isinstance(timestamp, six.string_types): timestamp = timeutils.parse_strtime(timestamp) self.timestamp = timestamp - if not request_id: - request_id = context.generate_request_id() - self.request_id = request_id if service_catalog: # Only include required parts of service_catalog @@ -154,25 +158,30 @@ class RequestContext(context.RequestContext): def to_dict(self): values = super(RequestContext, self).to_dict() - values.update({'user_id': self.user_id, - 'project_id': self.project_id, - 'is_admin': self.is_admin, - 'read_deleted': self.read_deleted, - 'roles': self.roles, - 'remote_address': self.remote_address, - 'timestamp': timeutils.strtime(self.timestamp), - 'request_id': self.request_id, - 'quota_class': self.quota_class, - 'user_name': self.user_name, - 'service_catalog': self.service_catalog, - 'project_name': self.project_name, - 'instance_lock_checked': self.instance_lock_checked}) + # FIXME(dims): defensive hasattr() checks need to be + # removed once we figure out why we are seeing stack + # traces + values.update({ + 'user_id': getattr(self, 'user_id', None), + 'project_id': getattr(self, 'project_id', None), + 'is_admin': getattr(self, 'is_admin', None), + 'read_deleted': getattr(self, 'read_deleted', 'no'), + 'roles': getattr(self, 'roles', None), + 'remote_address': getattr(self, 'remote_address', None), + 'timestamp': timeutils.strtime(self.timestamp) if hasattr( + self, 'timestamp') else None, + 'request_id': getattr(self, 'request_id', None), + 'quota_class': getattr(self, 'quota_class', None), + 'user_name': getattr(self, 'user_name', None), + 'service_catalog': getattr(self, 'service_catalog', None), + 'project_name': getattr(self, 'project_name', None), + 'instance_lock_checked': getattr(self, 'instance_lock_checked', + False) + }) return values @classmethod def from_dict(cls, values): - values.pop('user', None) - values.pop('tenant', None) return cls(**values) def elevated(self, read_deleted=None, overwrite=False): diff --git a/nova/tests/unit/test_context.py b/nova/tests/unit/test_context.py index 1d8324291943..2f0273b02ddf 100644 --- a/nova/tests/unit/test_context.py +++ b/nova/tests/unit/test_context.py @@ -165,3 +165,61 @@ class ContextTestCase(test.NoDBTestCase): overwrite=True) context.get_admin_context() self.assertIs(o_context.get_current(), ctx1) + + def test_convert_from_rc_to_dict(self): + ctx = context.RequestContext( + 111, 222, request_id='req-679033b7-1755-4929-bf85-eb3bfaef7e0b', + timestamp='2015-03-02T22:31:56.641629') + values2 = ctx.to_dict() + expected_values = {'auth_token': None, + 'domain': None, + 'instance_lock_checked': False, + 'is_admin': False, + 'project_id': 222, + 'project_domain': None, + 'project_name': None, + 'quota_class': None, + 'read_deleted': 'no', + 'read_only': False, + 'remote_address': None, + 'request_id': + 'req-679033b7-1755-4929-bf85-eb3bfaef7e0b', + 'resource_uuid': None, + 'roles': [], + 'service_catalog': [], + 'show_deleted': False, + 'tenant': 222, + 'timestamp': '2015-03-02T22:31:56.641629', + 'user': 111, + 'user_domain': None, + 'user_id': 111, + 'user_identity': '111 222 - - -', + 'user_name': None} + self.assertEqual(expected_values, values2) + + def test_convert_from_dict_then_to_dict(self): + values = {'user': '111', + 'user_id': '111', + 'tenant': '222', + 'project_id': '222', + 'domain': None, 'project_domain': None, + 'auth_token': None, + 'resource_uuid': None, 'read_only': False, + 'user_identity': '111 222 - - -', + 'instance_lock_checked': False, + 'user_name': None, 'project_name': None, + 'timestamp': '2015-03-02T20:03:59.416299', + 'remote_address': None, 'quota_class': None, + 'is_admin': True, + 'service_catalog': [], + 'read_deleted': 'no', 'show_deleted': False, + 'roles': [], + 'request_id': 'req-956637ad-354a-4bc5-b969-66fd1cc00f50', + 'user_domain': None} + ctx = context.RequestContext.from_dict(values) + self.assertEqual(ctx.user, '111') + self.assertEqual(ctx.tenant, '222') + self.assertEqual(ctx.user_id, '111') + self.assertEqual(ctx.project_id, '222') + values2 = ctx.to_dict() + self.assertEqual(values, values2)