insulate TroveContext from possible changes in oslo.context
Earlier code introduced in[1] aimed to handle the situation where a
context object serialized and sent over a message queue as a
dictionary is received by a version of code that doesn't necessarily
understand that context object.
That code relied on the fact that vars(TroveContext).keys() was a
dictionary that matched the full set of parameters that you can send
to the initializers for TroveContext() and RequestContext().
Recent changes in oslo.context [2] and [3] broke that
assumption. vars() on TroveContext provided a dictionary with the
internal representation(s) that are not the same as the kwargs in
RequestContext() and the params that TroveContext used to pop.
To get around this, new code introduced here uses
oslo_utils.reflection to determine all the possible kwargs that can be
sent to TroveContext, some of which are consumed by TroveContext and
the rest of which are sent along to RequestContext().
The construct in the earlier _remove_incompatible_context_args() also
modified kwargs.keys() by pop'ing values from it while iterating over
this. Python 3 takes a dim view of this and therefore some changes
have been made to accomodate this.
A unittest has been added to ensure that the from_dict() method
properly dumps stuff it doesn't know about. The test explicitly
verifies the warning generated when a bogus argument is eliminated.
[1] 24c5e8e244
[2] 2394cff0631944a9259bfe04925e444d9f817758
[3] f25543fcc792ebf155728a91fde06e8dc4e96cea
Change-Id: I477dd29e034295e770925091c4ac6268c22ae59b
Related-Bug:#1661790
This commit is contained in:
parent
55dbc74b30
commit
577d43b02d
|
@ -249,6 +249,18 @@
|
|||
"Class 'Commands' has no 'has' member",
|
||||
"Commands.params_of"
|
||||
],
|
||||
[
|
||||
"trove/common/context.py",
|
||||
"E1101",
|
||||
"Module 'inspect' has no 'getfullargspec' member",
|
||||
"TroveContext._remove_incompatible_context_args"
|
||||
],
|
||||
[
|
||||
"trove/common/context.py",
|
||||
"no-member",
|
||||
"Module 'inspect' has no 'getfullargspec' member",
|
||||
"TroveContext._remove_incompatible_context_args"
|
||||
],
|
||||
[
|
||||
"trove/common/extensions.py",
|
||||
"E1003",
|
||||
|
|
|
@ -20,9 +20,12 @@ Projects should subclass this class if they wish to enhance the request
|
|||
context or provide additional information in their specific WSGI pipeline.
|
||||
"""
|
||||
|
||||
|
||||
from oslo_context import context
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import reflection
|
||||
|
||||
from trove.common.i18n import _
|
||||
from trove.common import local
|
||||
from trove.common.serializable_notification import SerializableNotification
|
||||
|
||||
|
@ -34,15 +37,15 @@ class TroveContext(context.RequestContext):
|
|||
Stores information about the security context under which the user
|
||||
accesses the system, as well as additional request information.
|
||||
"""
|
||||
def __init__(self, **kwargs):
|
||||
self.limit = kwargs.pop('limit', None)
|
||||
self.marker = kwargs.pop('marker', None)
|
||||
self.service_catalog = kwargs.pop('service_catalog', None)
|
||||
self.user_identity = kwargs.pop('user_identity', None)
|
||||
self.instance_id = kwargs.pop('instance_id', None)
|
||||
|
||||
# TODO(esp): not sure we need this
|
||||
self.timeout = kwargs.pop('timeout', None)
|
||||
def __init__(self, limit=None, marker=None, service_catalog=None,
|
||||
user_identity=None, instance_id=None, timeout=None,
|
||||
**kwargs):
|
||||
self.limit = limit
|
||||
self.marker = marker
|
||||
self.service_catalog = service_catalog
|
||||
self.user_identity = user_identity
|
||||
self.instance_id = instance_id
|
||||
self.timeout = timeout
|
||||
super(TroveContext, self).__init__(**kwargs)
|
||||
|
||||
if not hasattr(local.store, 'context'):
|
||||
|
@ -65,16 +68,20 @@ class TroveContext(context.RequestContext):
|
|||
|
||||
@classmethod
|
||||
def _remove_incompatible_context_args(cls, values):
|
||||
LOG.debug("Running in unsafe mode and ignoring incompatible context.")
|
||||
return values
|
||||
realvalues = {}
|
||||
|
||||
args = (reflection.get_callable_args(context.RequestContext.__init__) +
|
||||
reflection.get_callable_args(TroveContext.__init__))
|
||||
|
||||
context_keys = vars(cls()).keys()
|
||||
for dict_key in values.keys():
|
||||
if dict_key not in context_keys:
|
||||
LOG.debug("Argument being removed before instantiating "
|
||||
"TroveContext object - %s" % dict_key)
|
||||
values.pop(dict_key, None)
|
||||
return values
|
||||
if dict_key in args:
|
||||
realvalues[dict_key] = values.get(dict_key)
|
||||
else:
|
||||
LOG.warning(_("Argument being removed before instantiating "
|
||||
"TroveContext object - %(key)s = %(value)s"),
|
||||
{'key': dict_key, 'value': values.get(dict_key)})
|
||||
|
||||
return realvalues
|
||||
|
||||
@classmethod
|
||||
def from_dict(cls, values):
|
||||
|
|
|
@ -13,6 +13,7 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
#
|
||||
import mock
|
||||
from mock import Mock
|
||||
|
||||
from testtools.matchers import Equals, Is
|
||||
|
@ -63,3 +64,24 @@ class TestTroveContext(trove_testtools.TestCase):
|
|||
self.assertThat(n_dict.get('notification_classname'),
|
||||
Equals('trove.common.notification.'
|
||||
'DBaaSInstanceCreate'))
|
||||
|
||||
def test_create_with_bogus(self):
|
||||
with mock.patch('trove.common.context.LOG') as mock_log:
|
||||
ctx = context.TroveContext.from_dict(
|
||||
{'user': 'test_user_id',
|
||||
'request_id': 'test_req_id',
|
||||
'tenant': 'abc',
|
||||
'blah_blah': 'blah blah'})
|
||||
mock_log.warning.assert_called()
|
||||
mock_log.warning.assert_called_with('Argument being removed '
|
||||
'before instantiating '
|
||||
'TroveContext object - '
|
||||
'%(key)s = %(value)s',
|
||||
{'value': 'blah blah',
|
||||
'key': 'blah_blah'})
|
||||
self.assertThat(ctx.user, Equals('test_user_id'))
|
||||
self.assertThat(ctx.request_id, Equals('test_req_id'))
|
||||
self.assertThat(ctx.tenant, Equals('abc'))
|
||||
self.assertThat(ctx.limit, Is(None))
|
||||
self.assertThat(ctx.marker, Is(None))
|
||||
self.assertThat(ctx.service_catalog, Is(None))
|
||||
|
|
Loading…
Reference in New Issue