[openstack-dev] [Oslo][Designate][Congress] Problem with from_dict overrides and new oslo.context release

Doug Hellmann doug at doughellmann.com
Mon Oct 9 19:07:42 UTC 2017


Excerpts from Ben Nemec's message of 2017-10-09 12:03:24 -0500:
> Hi,
> 
> I brought up the fix[1] I proposed for [2] in the Oslo meeting today, 
> and it led to some good discussion that we wanted to take to a broader 
> audience.  In particular, the projects that are affected by the bug.
> 
> The oslo.context change I proposed would essentially obsolete the 
> from_dict function in the Context object since it would obligate us to 
> support all the keys in to_dict in the constructor.  This unfortunately 
> implies some rather rigid constraints on the Context object.  We could 
> essentially never remove any parameters from the constructor (which at 
> some point we want to do for things like "tenant" that are deprecated 
> but still present) or risk breaking if someone passed in parameters from 
> an old to_dict call.
> 
> The other proposal was to rewrite the existing naive from_dict overrides 
> in the affected projects to function more like the from_dict in the base 
> class where it explicitly handles only the keys that the constructor 
> will recognize.[3]  One benefit of this approach is that it would allow 
> the removal of some previous workarounds[4].  This is also cleaner from 
> an API perspective as it doesn't impose any new requirements on the 
> oslo.context API.  The obvious drawback is that it requires 
> project-specific fixes in the affected projects.  We would, of course, 
> be happy to help with those fixes though.
> 
> So those are the options we've discussed and my thoughts on them. 
> Please feel free to weigh in with any other input you may have.  Thanks.
> 
> -Ben
> 
> 1: https://review.openstack.org/509919
> 2: https://launchpad.net/bugs/1721432
> 3: 
> https://github.com/openstack/oslo.context/blob/master/oslo_context/context.py#L371
> 4: 
> https://github.com/openstack/designate/blob/46de766e513cfb97cbfc50b001734e02711517e4/designate/context.py#L42
> 

I originally objected to [1] because it seemed like the from_dict()
API was the right place to make the change, and that projects should
be using that class method instead of the constructor if they're
creating contexts from dicts full of data where they can't be sure
of the keys. The current implementation of from_dict() merges keyword
arguments and a dictionary before passing the results to the
constructor, but we could also have it do name translation, strip
unknown arguments (with a warning), etc.

Long term, I think this makes the context API easier to understand
because the constructor arguments are the expected names and all
of the munging logic is isolated in the alternate constructor. Short
term, it's going to be a little more work to fix the current breakage.
I will help with those changes.

Doug



More information about the OpenStack-dev mailing list