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