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.porr...@dektech.com.au >> <mailto:pasquale.porr...@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.porr...@dektech.com.au >> <mailto:pasquale.porr...@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@lists.openstack.org >> <mailto:OpenStack-dev@lists.openstack.org> >> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >> > _______________________________________________ >> > OpenStack-dev mailing list >> > OpenStack-dev@lists.openstack.org >> <mailto:OpenStack-dev@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@lists.openstack.org >> <mailto:OpenStack-dev@lists.openstack.org> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >> >> _______________________________________________ >> OpenStack-dev mailing list >> OpenStack-dev@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@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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev