[openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches
Monty Taylor
mordred at inaugust.com
Mon Nov 13 16:30:10 UTC 2017
On 11/13/2017 09:06 AM, David Shrewsbury wrote:
> Hi!
>
> On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor <mordred at inaugust.com
> <mailto:mordred at inaugust.com>> wrote:
>
> Hey everybody,
>
> You may have noticed a giant patch series up:
>
> https://review.openstack.org/#/q/topic:merge-shade
> <https://review.openstack.org/#/q/topic:merge-shade>
>
> <snip>
>
> One thing I don't see covered here is the current set of Ansible module
> tests. Unless I've missed it,
> I don't see any migration of those, nor any reference of the plan for
> them in the future. I know that
> we were waiting for Zuulv3 to do some cool github integration things.
> And since the modules import
> shade, not this code, we won't be breaking them. But, what's the plan
> for that?
That's a great question. I actually did add the ansible functional tests
to the "add zuulv3 jobs" patch - but as you say, those aren't really
testing anything yet since the ansible modules do "import shade"
That brings up a few really good things that should be pointed out:
1) The intent is to turn the code in the shade repo into a thin compat
shim that imports python-openstacksdk and subclasses/wraps the sdk
object. That'll let us fix some of the interface mistakes we made in
shade but are stuck with for compat reasons in the sdk version of the
code ... but can still keep the old defaults/behavior in the shade code.
For instance ... we've learned that fetching extra_specs on flavors in
shade was ... stupid. We can have the sdk version NOT fetch by default,
then in shade do:
def list_flavors(self, fetch_extra_specs=True):
return super(OpenStackCloud, self).list_flavors(
fetch_extra_specs=fetch_extra_specs)
With this in place, it should mean that shade users can continue on
without being broken.
2) Once we have a release of openstacksdk that has the shade code in a
place we're happy with, we should update the ansible modules to do
import openstack instead of import shade
3) Cross-testing shade, os-client-config and openstacksdk is essential,
so that we can make sure we're not breaking anything as we work on
compat shims. The same goes with python-openstackclient, but current v3
and the v4 branch dean is working on. We should probably add a shared
change queue to the zuul v3 config for each of them. We can't add
python-openstackclient to that shared queue since I'm pretty sure it's
in the integrated-gate change queue. We COULD add shade, sdk and occ to
the integrated-gate queue ... but that might slow us down at the moment,
so maybe once it's all tied in together like we want it ...
4) openstack.cloud is the wrong home for the shade code. It's just there
for expedience sake for now (let's not change TOO many things all at
once) I'm *pretty* sure the shade methods want to just live on the
Connection object, so that we wind up with:
conn = openstack.connection.Connection(cloud='example')
conn.list_servers() # shade version
conn.compute.servers() # sdk version
conn.compute.get('/servers') # REST version
We could alternately put it as a sub-resource, but since the shade
methods are intended to be the 'easy' methods, doing this:
conn = openstack.connection.Connection(cloud='example')
conn.cloud.list_servers() # shade version
conn.compute.servers() # sdk version
conn.compute.get('/servers') # REST version
feels wrong. Maybe keeping the code in openstack/cloud is fine and just
making it a mixin class (kind of like how we do in shade today with the
normalize methods) would allow for some better source-code organization.
I also think that the OpenStackCloud/OperatorCloud split from shade
wound up being more confusing than helpful.
Finally - while I'm rambling here ... after we finish the Resource ->
Resource2 migration, I'd love to ponder whether or not we can make
Resource2 be a subclass of munch.Munch. shade needs to be able to return
objects that are directly json serializable (so that the ansible layer
works easily/well) - it would be awesome if we didn't have to shove a
to_dict() into every return on that layer. I haven't dug in to the magic
guts of Resource2 fully, so I'm not 100% sure doing that will work.
Since Resource2 is already doing data model definition via object
parameters the munch part may not be super useful. We'll have to think
about how we can pass through something so that shade users get things
that are isinstance(munch.Munch) though - which is maybe a metaclass if
we can't do it directly at the Resource layer.
Monty
More information about the OpenStack-dev
mailing list