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

Pasquale Porreca pasquale.porreca at dektech.com.au
Wed Dec 17 17:18:06 UTC 2014


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




More information about the OpenStack-dev mailing list