[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