[openstack-dev] [nova] Complexity check and v2 API

Pasquale Porreca pasquale.porreca at dektech.com.au
Thu Dec 18 13:32:32 UTC 2014


I created a bug  report and proposed a fix for this issue:

https://bugs.launchpad.net/nova/+bug/1403586

@Matthew Gilliard: I added you as reviewer for my patch, since you asked
for it.

Thanks to anyone that will want to review the bug report and the patch.

On 12/18/14 09:33, Pasquale Porreca wrote:
> Yes, for v2.1 there is not this problem, moreover v2.1 corresponding
> server.py has much lower complexity than v2 one.
>
> On 12/17/14 20:10, Christopher Yeoh wrote:
>> Hi,
>>
>> Given the timing (no spec approved) it sounds like a v2.1 plus
>> microversions (just merging) with no v2 changes at all.
>>
>> The v2.1 framework is more flexible and you should need no changes to
>> servers.py at all as there are hooks for adding extra parameters in
>> separate plugins. There are examples of this in the v3 directory
>> which is really v2.1 now.
>>
>> Chris
>> On Thu, 18 Dec 2014 at 3:49 am, Pasquale Porreca
>> <pasquale.porreca at dektech.com.au
>> <mailto:pasquale.porreca at dektech.com.au>> wrote:
>>
>>     Thank you for the answer.
>>
>>     my API proposal won't be merged in kilo release since the
>>     deadline for
>>     approval is tomorrow, so I may propose the fix to lower the
>>     complexity
>>     in another way, what do you think about a bug fix?
>>
>>     On 12/17/14 18:05, Matthew Gilliard wrote:
>>     > Hello Pasquale
>>     >
>>     >   The problem is that you are trying to add a new if/else
>>     branch into
>>     > a method which is already ~250 lines long, and has the highest
>>     > complexity of any function in the nova codebase. I assume that you
>>     > didn't contribute much to that complexity, but we've recently
>>     added a
>>     > limit to stop it getting any worse. So, regarding your 4
>>     suggestions:
>>     >
>>     >     1/ As I understand it, v2.1 should be the same as v2 at the
>>     > moment, so they need to be kept the same
>>     >     2/ You can't ignore it - it will fail CI
>>     >     3/ No thank you. This limit should only ever be lowered :-)
>>     >     4/ This is 'the right way'. Your suggestion for the
>>     refactor does
>>     > sound good.
>>     >
>>     > I suggest a single patch that refactors and lowers the limit in
>>     > tox.ini.  Once you've done that then you can add the new
>>     parameter in
>>     > a following patch. Please feel free to add me to any patches you
>>     > create.
>>     >
>>     > Matthew
>>     >
>>     >
>>     >
>>     > On Wed, Dec 17, 2014 at 4:18 PM, Pasquale Porreca
>>     > <pasquale.porreca at dektech.com.au
>>     <mailto:pasquale.porreca at dektech.com.au>> wrote:
>>     >> Hello
>>     >>
>>     >> I am working on an API extension that adds a parameter on
>>     create server
>>     >> call; to implement the v2 API I added few lines of code to
>>     >> nova/api/openstack/compute/servers.py
>>     >>
>>     >> In particular just adding something like
>>     >>
>>     >> new_param = None
>>     >> if self.ext_mgr.is_loaded('os-new-param'):
>>     >>     new_param = server_dict.get('new_param')
>>     >>
>>     >> leads to a pep8 fail with message 'Controller.create' is too
>>     complex (47)
>>     >> (Note that in tox.ini the max complexity is fixed to 47 and
>>     there is a note
>>     >> specifying 46 is the max complexity present at the moment).
>>     >>
>>     >> It is quite easy to make this test pass creating a new method
>>     just to
>>     >> execute these lines of code, anyway all other extensions are
>>     handled in that
>>     >> way and one of most important stylish rule states to be
>>     consistent with
>>     >> surrounding code, so I don't think a separate function is the
>>     way to go
>>     >> (unless it implies a change in how all other extensions are
>>     handled too).
>>     >>
>>     >> My thoughts on this situation:
>>     >>
>>     >> 1) New extensions should not consider v2 but only v2.1, so
>>     that file should
>>     >> not be touched
>>     >> 2) Ignore this error and go on: if and when the extension will
>>     be merged the
>>     >> complexity in tox.ini will be changed too
>>     >> 3) The complexity in tox.ini should be raised to allow new v2
>>     extensions
>>     >> 4) The code of that module should be refactored to lower the
>>     complexity
>>     >> (i.e. move the load of each extension in a separate function)
>>     >>
>>     >> I would like to know if any of my point is close to the
>>     correct solution.
>>     >>
>>     >> --
>>     >> Pasquale Porreca
>>     >>
>>     >> DEK Technologies
>>     >> Via dei Castelli Romani, 22
>>     >> 00040 Pomezia (Roma)
>>     >>
>>     >> Mobile +39 3394823805
>>     >> Skype paskporr
>>     >>
>>     >>
>>     >> _______________________________________________
>>     >> OpenStack-dev mailing list
>>     >> OpenStack-dev at lists.openstack.org
>>     <mailto:OpenStack-dev at lists.openstack.org>
>>     >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>     >>
>>     > _______________________________________________
>>     > OpenStack-dev mailing list
>>     > OpenStack-dev at lists.openstack.org
>>     <mailto:OpenStack-dev at lists.openstack.org>
>>     > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>     --
>>     Pasquale Porreca
>>
>>     DEK Technologies
>>     Via dei Castelli Romani, 22
>>     00040 Pomezia (Roma)
>>
>>     Mobile +39 3394823805
>>     Skype paskporr
>>
>>
>>     _______________________________________________
>>     OpenStack-dev mailing list
>>     OpenStack-dev at lists.openstack.org
>>     <mailto:OpenStack-dev at lists.openstack.org>
>>     http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
> -- 
> Pasquale Porreca
>
> DEK Technologies
> Via dei Castelli Romani, 22
> 00040 Pomezia (Roma)
>
> Mobile +39 3394823805
> Skype paskporr
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-- 
Pasquale Porreca

DEK Technologies
Via dei Castelli Romani, 22
00040 Pomezia (Roma)

Mobile +39 3394823805
Skype paskporr

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141218/91754a12/attachment.html>


More information about the OpenStack-dev mailing list