Open Stack

On Tue, Jan 06, 2015 at 07:31:19AM -0500, Jay Pipes wrote:
> On 01/06/2015 06:25 AM, Chen CH Ji wrote:
> >Based on nova-specs api-microversions.rst
> >we support following function definition format, but it violate the
> >hacking rule pep8 F811 because duplicate function definition
> >we should use #noqa for them , but considering microversion may live for
> >long time ,
> >keep adding #noqa may be a little bit ugly, can anyone suggest a good
> >solution for it ? thanks
> >
> > >   @api_version(min_version='2.1')
> > >   def _version_specific_func(self, req, arg1):
> > >      pass
> > >
> > >   @api_version(min_version='2.5')
> > >   def _version_specific_func(self, req, arg1):
> > >      pass
> 
> Hey Kevin,
> 
> This was actually one of my reservations about the proposed microversioning
> implementation -- i.e. having functions that are named exactly the same,
> only decorated with the microversioning notation. It kinda reminds me of the
> hell of debugging C++ code that uses STL: how does one easily know which
> method one is in when inside a debugger?
> 
> That said, the only other technique we could try to use would be to not use
> a decorator and instead have a top-level dispatch function that would
> inspect the API microversion (only when the API version makes a difference
> to the output or input of that function) and then dispatch the call to a
> helper method that had the version in its name.
> 
> So, for instance, let's say you are calling the controller's GET
> /$tenant/os-hosts method, which happens to get routed to the
> nova.api.openstack.compute.contrib.hosts.HostController.index() method. If
> you wanted to modify the result of that method and the API microversion is
> at 2.5, you might do something like:
> 
>  def index(self, req):
>      req_api_ver = utils.get_max_requested_api_version(req)
>      if req_api_ver == (2, 5):
>          return self.index_2_5(req)
>      return self.index_2_1(req)
> 
>  def index_2_5(self, req):
>      results = self.index_2_1(req)
>      # Replaces 'host' with 'host_name'
>      for result in results:
>          result['host_name'] = result['host']
>          del result['host']
>      return results
> 
>  def index_2_1(self, req):
>      # Would be a rename of the existing index() method on
>      # the controller....

Yes, I'd much prefer to see us use version numbers in the method names
like this and not have identically named methods. With some cleverness
we could perhaps avoid the need for the repetative 'index' impl and just
auto-detect the versioned method names when needed.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|