Re: [openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches

2017-11-13 Thread David Shrewsbury 
Hi!

Thanks for the explanations.

On Mon, Nov 13, 2017 at 11:30 AM, Monty Taylor  wrote:



I also think that the OpenStackCloud/OperatorCloud split from shade wound
> up being more confusing than helpful.
>

Yep. At the time, the classes were doing different auth things, so it at
least made a little sense. Now, not so much.


-Dave
-- 
David Shrewsbury (Shrews)
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches

2017-11-13 Thread Monty Taylor 

On 11/13/2017 09:06 AM, David Shrewsbury wrote:

Hi!

On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor > wrote:


Hey everybody,

You may have noticed a giant patch series up:

https://review.openstack.org/#/q/topic:merge-shade




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

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches

2017-11-13 Thread David Shrewsbury 
Hi!

On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor  wrote:

> Hey everybody,
>
> You may have noticed a giant patch series up:
>
>   https://review.openstack.org/#/q/topic:merge-shade
>
> 

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?

-- 
David Shrewsbury (Shrews)
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches

2017-11-13 Thread Monty Taylor 

Hey everybody,

You may have noticed a giant patch series up:

  https://review.openstack.org/#/q/topic:merge-shade

It's a big beastie, so I thought a synopsis of what's going would be 
useful. Also, there's a pile of patches towards the end that we want to 
squash before landing but which have been pushed up in smaller patches 
because reading it all as one patch would be unpossible - but reworking 
the unittest mocks for each step would be wasted energy.


Any of the patches in this email that are green from Zuul are ready for 
review/merge.


The easy patches:

These next patches are easy because they're just mechanical - nothing 
really will have changed in SDK because of them.


https://review.openstack.org/#/c/518128 - Merge shade and 
os-client-config into the tree


You can't see the whole picture in gerrit. This merges in shade and 
os-client-config, including their entire git history. Most of that merge 
got pushed up to a throwaway branch so that the review in gerrit would 
be a single patch containing the merge. It's best to just grab the repo 
at that branch and take a look.


It's also important to note here that I'm pretty sure openstack.cloud is 
not actually where shade wants to live and that openstack.config needs a 
refactor - but we can do that after this stack, as the stack is too big 
already.


https://review.openstack.org/#/c/518129/ - Merge in recent fixes from 
the shade repo


Merges patches that already landed in shade between the time I made the 
first merge commit and proposed it. No need to actually review the 
content itself.


https://review.openstack.org/#/c/518130 - Add jobs for Zuul v3

Adds functional zuulv3 native test jobs that match what shade was doing. 
These run both shade and sdk's functional tests (they're one set of tests)


https://review.openstack.org/#/c/518946 - Fix magnum functional test

Fixes the previous one.

https://review.openstack.org/#/c/518131 - Handle glance image pagination 
links better
https://review.openstack.org/#/c/518132 - Fix regression for 
list_router_interfaces
https://review.openstack.org/#/c/518133 - Support filtering servers in 
list_servers using arbitrary parameters


Patches landed in shade in the interim. No need to review content.

Once those are landed we're ready for some fun:

https://review.openstack.org/#/c/518799 - Migrate to testtools for 
functional tests


Reworks the functional base class to use the testtools based functional 
base class from shade, along with the logger fakes and whatnot. The 
biggest change here is the move to using setUp/tearDown instead of 
setUpClass/tearDownClass.


This patch is annoyingly large, but it's also important so that we have 
one consistent approach to these things across the combined codebase.


https://review.openstack.org/#/c/519029 - Rework config and rest layers

The squahsed/rollup patch. It can be viewed broken out by looking at the 
other patches that have https://review.openstack.org/#/c/518799 as a parent.


This is where all of the invasive breaking-changes happen. Most notably 
stepping away from Profile, Session and Authenticator in favor of using 
openstack.config/os-client-config and the Adapter class from 
keystoneauth so that we can rely on ksa for discovery and whatnot. It 
also adds a pure-rest interface on each service, removes the plugin 
entrypoints structure and and renames 4 of the service objects to match 
their official service-type (although it also keeps aliases so that 
shouldn't cause too much carnage for people)


Removal of profile/authenticator is the biggest user-visible breaking 
change, and it's not ACTUALLY removed in this patch (although it was at 
one point) We have to keep Profile around long enough to remove its use 
from OSC - including osc3, since OSC is used to set up resources as part 
of devstack. It might be worthwhile to consider fleshing out the 
backwards compat for allowing profiles to be passed in for a short time 
since it's there now.


There's still a little more plumbing to do before we can start working 
on using this layer under the shade code - most notably we need to plumb 
in the task manager ... but to my knowledge this stack works with Dean's 
OSC4 branch so we should be in good shape.


https://review.openstack.org/#/c/501438 - Add notes about moving forward

A document with some thoughts - some of which are a good idea and some 
not - about next steps and what we need to get done.


Thanks for the patience in reading these ... they're SUPER long. In many 
cases it's about letting the test suites let us know they're ok.


Monty

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev