Open Stack

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
>