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

Pasquale Porreca pasquale.porreca at dektech.com.au
Thu Dec 18 08:33:37 UTC 2014


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

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


More information about the OpenStack-dev mailing list