Open Stack

On Tue, 25 Feb 2014 14:47:16 -0800
Dan Smith <dms at danplanet.com> wrote:
> > Yeah, so objects is the big one here.
> 
> Objects, and everything else. With no-db-compute we did it for a
> couple cycles, then objects, next it will be retooling flows to
> conductor, then dealing with tasks, talking to gantt, etc. It's not
> going to end any time soon.

So I think there's some areas where I think the burden of making
changes to two APIs is very low. Take for example adding object support
for unrescue:

diff ec78b42d7b7e9da99ba063cccc8a4f6d0aa7c8e5^..ec78b42d7b7e9da99ba063cccc8a4f6d0aa7c8e5 | diffstat
 api/openstack/compute/contrib/rescue.py    |    2 +-
 api/openstack/compute/plugins/v3/rescue.py |    3 ++-
 compute/manager.py                         |   14 +++++++-------
 compute/rpcapi.py                          |   12 ++++++++----
 tests/compute/test_compute.py              |    8 +++++---
 tests/compute/test_rpcapi.py               |    2 +-
 6 files changed, 24 insertions(+), 17 deletions(-)

And the delta for the v2/v3 parts is:

diff --git a/nova/api/openstack/compute/contrib/rescue.py b/nova/api/openstack/compute/con
index fe31f2c..0233be2 100644
--- a/nova/api/openstack/compute/contrib/rescue.py
+++ b/nova/api/openstack/compute/contrib/rescue.py
@@ -75,7 +75,7 @@ class RescueController(wsgi.Controller):
         """Unrescue an instance."""
         context = req.environ["nova.context"]
         authorize(context)
-        instance = self._get_instance(context, id)
+        instance = self._get_instance(context, id, want_objects=True)
         try:
             self.compute_api.unrescue(context, instance)
         except exception.InstanceInvalidState as state_error:
diff --git a/nova/api/openstack/compute/plugins/v3/rescue.py b/nova/api/openstack/compute/
index 5ae876b..66b4c17 100644
--- a/nova/api/openstack/compute/plugins/v3/rescue.py
+++ b/nova/api/openstack/compute/plugins/v3/rescue.py
@@ -77,7 +77,8 @@ class RescueController(wsgi.Controller):
         """Unrescue an instance."""
         context = req.environ["nova.context"]
         authorize(context)
-        instance = common.get_instance(self.compute_api, context, id)
+        instance = common.get_instance(self.compute_api, context, id,
+                                       want_objects=True)
         try:
             self.compute_api.unrescue(context, instance)
         except exception.InstanceInvalidState as state_error:

eg a one line trivial change in a patch with
 6 files changed, 24 insertions(+), 17 deletions(-)

So in those specific cases I think the v2/v3 dual maintenance burden is very low.

But there are also other cases (such as some of the flavors apis) where
the extension basically does:

1. parse incoming data
2. call some flavor code
3. get what is returned and mangle it into a temporary data structure
4. format data for returning to the user

Now 1 and 4 are very v2 and v3 API specific. But 2 and 3 tend to be more
generic (this is not always the case with error paths etc) and do need to be
changed with object transition (and perhaps some of the other changes you are
talking about). eg foo['aaa'] -> foo.aaa. Or adding want_objects=True to 
a method.

Now I still maintain that trying to squeeze both v2 and v3 parsing/formatting
into the same file/method is the wrong thing to do. But we could possibly
expand on nova/api/openstack/common.py and push cases where we can cases of
2 and 3 into it as common methods which the v2/v3 apis call down into.

This would reduce the amount of duplication which is required (I doubt
we could remove all duplication though) and whether its worth it for say
the rescue example is debatable. But for those cases you'd only need to make
the modification in one file.

However we would still have unittest and and tempest burden (I don't see
how we avoid that if we are ever going to fix the v2 API).

Chris