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

Chen CH Ji jichenjc at cn.ibm.com
Wed Jan 28 18:44:23 UTC 2015


change or disable F811 might be too huge, so add #noqa might be best way we
have now unless we change the code logic ..

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



From:	Matthew Gilliard <matthew.gilliard at gmail.com>
To:	"OpenStack Development Mailing List (not for usage questions)"
            <openstack-dev at lists.openstack.org>
Date:	01/28/2015 06:32 PM
Subject:	Re: [openstack-dev] [nova] Questions on pep8 F811 hacking check
            for	microversion



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

  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 gChristopher 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

__________________________________________________________________________
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/5973a0e0/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/5973a0e0/attachment.gif>


More information about the OpenStack-dev mailing list