[openstack-dev] [nova] Questions on pep8 F811 hacking check for microversion

Matthew Gilliard matthew.gilliard at gmail.com
Wed Jan 28 17:30:37 UTC 2015


F811 is not part of our hacking lib - it's in flake8.  As far as I know,
it's not possible to selectively disable that for particular files or
methods.  And as mentioned earlier in the list and when I asked in
#openstack-dev the feeling was that we don't want to disable F811 globally
because it's a useful check. So I think we have to choose between:

* Continuing to use #noqa
* Disabling F811 globally
* Modifying flake8
* Something else I haven't thought of

  Matthew

On Wed, Jan 28, 2015 at 7:43 AM, Chen CH Ji <jichenjc at cn.ibm.com> wrote:

> Is there a way to overwrite the rule in our hacking (not familiar with it
> ...)?
> if so ,maybe we can do as suggested to avoid 811 for the class which has
> Microversion definition? Thanks
>
> Best Regards!
>
> Kevin (Chen) Ji 纪 晨
>
> Engineer, zVM Development, CSTL
> Notes: Chen CH Ji/China/IBM at IBMCN   Internet: jichenjc at cn.ibm.com
> Phone: +86-10-82454158
> Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian District,
> Beijing 100193, PRC
>
> [image: Inactive hide details for Christopher Yeoh ---01/28/2015 09:37:00
> AM---On Tue, 06 Jan 2015 07:31:19 -0500 Jay Pipes <jaypipes at g]Christopher
> Yeoh ---01/28/2015 09:37:00 AM---On Tue, 06 Jan 2015 07:31:19 -0500 Jay
> Pipes <jaypipes at gmail.com> wrote:
>
> From: Christopher Yeoh <cbkyeoh at gmail.com>
> To: openstack-dev at lists.openstack.org
> Date: 01/28/2015 09:37 AM
> Subject: Re: [openstack-dev] [nova] Questions on pep8 F811 hacking check
> for microversion
> ------------------------------
>
>
>
> On Tue, 06 Jan 2015 07:31:19 -0500
> Jay Pipes <jaypipes at gmail.com> 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....
> >
>
> So having to manually add switching code everything we have an API
> patch I think is not only longer and more complicated but more error
> prone when updating. If we change something at the core in the future it
> means changing all the microversioned code rather than just the
> switching architecture at the core of wsgi.
>
>
> > Another option would be to use something like JSON-patch to determine
> > the difference between two output schemas and automatically translate
> > one to another... but that would be a huge effort.
> >
> > That's the only other way I can think of besides disabling F811,
> > which I really would not recommend, since it's a valuable safeguard
> > against duplicate function names (especially duplicated test methods).
>
> So I don't think we need to disable F811 in general - why not just
> disable it for any method with the api_version decorator? On those ones
> we can do checks on what is passed to api_version which will help
> verify that there hasn't been a typo to an api_version decorator.
>
> Chris
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150128/f8b452b4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150128/f8b452b4/attachment.gif>


More information about the OpenStack-dev mailing list