Hi Ryan, Keeping in mind that 'mistral-lib' must not depend on ‘mistral’ below are my comments: I think porting keystone utils over to mistral-lib is OK, I don’t see any other options (‘mistral’ will depend on ‘mistral-lib’ but not the way around) Porting the entire mistral.context is OK too for the same reason. Porting the entire exceptions.py module is OK. But.. All general exceptions not related to actions should not be under ‘actions/api’ because this package should contain only stuff needed for implementing new actions. I would suggest we move all the exceptions into ‘mistral_lib/exceptions.py’ but keep ActionException (and other possible exceptions inherited from it) in ‘mistral_lib/actions/api.py”. That way the design would stay clean. As a rule of thumb: we need to keep under ‘api’ as little as possible, only that stuff that is really supposed to be stable and hence can be treated as API.
What do you think? Any other comments are very welcome. Renat Akhmerov @Nokia > 8 авг. 2016 г., в 21:33, Ryan Brady <[email protected]> написал(а): > > In accordance with the spec[1], I started a patch[2] to port security related > items from mistral to mistral-lib. This may not be the right way to approach > this task and I'm hoping the patch provides a means to illustrate the problem > and starts a discussion on the right solution. > > A custom action that creates a client that requires keystone auth will need > to get an endpoint for a given project to create a client object, so I ported > over the utility class[3] that deals with keystone. That utility class > requires the mistral.context. > > I started looking at the context requirements from two separate points of > view: > - create a security context in mistral lib that could be an attribute in the > mistral context > - port entire mistral context over > > When I looked at the attributes[4] currently in the mistral.context, most if > not all of them seem to be security related anyway. I chose to port the > entire context over, but that also required dependencies on 4 threading > utility methods[5] and mistral.exceptions[6], so those were also ported over. > > I would appreciate any feedback or discussion on the current proposed design. > > Thanks, > > Ryan > > > [1] > https://specs.openstack.org/openstack/mistral-specs/specs/newton/approved/mistral-custom-actions-api.html > > <https://specs.openstack.org/openstack/mistral-specs/specs/newton/approved/mistral-custom-actions-api.html> > > [2] https://review.openstack.org/#/c/352435/ > <https://review.openstack.org/#/c/352435/> > > [3] > https://github.com/openstack/mistral/blob/master/mistral/utils/openstack/keystone.py > > <https://github.com/openstack/mistral/blob/master/mistral/utils/openstack/keystone.py> > > [4] > https://github.com/openstack/mistral/blob/master/mistral/context.py#L76-L87 > <https://github.com/openstack/mistral/blob/master/mistral/context.py#L76-L87> > > [5] > https://github.com/openstack/mistral/blob/master/mistral/utils/__init__.py#L49-L94 > > <https://github.com/openstack/mistral/blob/master/mistral/utils/__init__.py#L49-L94> > > [6] https://github.com/openstack/mistral/blob/master/mistral/exceptions.py > <https://github.com/openstack/mistral/blob/master/mistral/exceptions.py> > > -- > Ryan Brady > Cloud Engineering > [email protected] <mailto:[email protected]> > 919.890.8925 > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
