Hi,<br><br>Given the timing (no spec approved) it sounds like a v2.1 plus microversions (just merging) with no v2 changes at all.<br><br>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.<br><br>Chris<br><div class="gmail_quote">On Thu, 18 Dec 2014 at 3:49 am, Pasquale Porreca <<a href="mailto:pasquale.porreca@dektech.com.au">pasquale.porreca@dektech.com.au</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thank you for the answer.<br>
<br>
my API proposal won't be merged in kilo release since the deadline for<br>
approval is tomorrow, so I may propose the fix to lower the complexity<br>
in another way, what do you think about a bug fix?<br>
<br>
On 12/17/14 18:05, Matthew Gilliard wrote:<br>
> Hello Pasquale<br>
><br>
> The problem is that you are trying to add a new if/else branch into<br>
> a method which is already ~250 lines long, and has the highest<br>
> complexity of any function in the nova codebase. I assume that you<br>
> didn't contribute much to that complexity, but we've recently added a<br>
> limit to stop it getting any worse. So, regarding your 4 suggestions:<br>
><br>
> 1/ As I understand it, v2.1 should be the same as v2 at the<br>
> moment, so they need to be kept the same<br>
> 2/ You can't ignore it - it will fail CI<br>
> 3/ No thank you. This limit should only ever be lowered :-)<br>
> 4/ This is 'the right way'. Your suggestion for the refactor does<br>
> sound good.<br>
><br>
> I suggest a single patch that refactors and lowers the limit in<br>
> tox.ini. Once you've done that then you can add the new parameter in<br>
> a following patch. Please feel free to add me to any patches you<br>
> create.<br>
><br>
> Matthew<br>
><br>
><br>
><br>
> On Wed, Dec 17, 2014 at 4:18 PM, Pasquale Porreca<br>
> <<a href="mailto:pasquale.porreca@dektech.com.au" target="_blank">pasquale.porreca@dektech.com.<u></u>au</a>> wrote:<br>
>> Hello<br>
>><br>
>> I am working on an API extension that adds a parameter on create server<br>
>> call; to implement the v2 API I added few lines of code to<br>
>> nova/api/openstack/compute/<u></u>servers.py<br>
>><br>
>> In particular just adding something like<br>
>><br>
>> new_param = None<br>
>> if self.ext_mgr.is_loaded('os-<u></u>new-param'):<br>
>> new_param = server_dict.get('new_param')<br>
>><br>
>> leads to a pep8 fail with message 'Controller.create' is too complex (47)<br>
>> (Note that in tox.ini the max complexity is fixed to 47 and there is a note<br>
>> specifying 46 is the max complexity present at the moment).<br>
>><br>
>> It is quite easy to make this test pass creating a new method just to<br>
>> execute these lines of code, anyway all other extensions are handled in that<br>
>> way and one of most important stylish rule states to be consistent with<br>
>> surrounding code, so I don't think a separate function is the way to go<br>
>> (unless it implies a change in how all other extensions are handled too).<br>
>><br>
>> My thoughts on this situation:<br>
>><br>
>> 1) New extensions should not consider v2 but only v2.1, so that file should<br>
>> not be touched<br>
>> 2) Ignore this error and go on: if and when the extension will be merged the<br>
>> complexity in tox.ini will be changed too<br>
>> 3) The complexity in tox.ini should be raised to allow new v2 extensions<br>
>> 4) The code of that module should be refactored to lower the complexity<br>
>> (i.e. move the load of each extension in a separate function)<br>
>><br>
>> I would like to know if any of my point is close to the correct solution.<br>
>><br>
>> --<br>
>> Pasquale Porreca<br>
>><br>
>> DEK Technologies<br>
>> Via dei Castelli Romani, 22<br>
>> 00040 Pomezia (Roma)<br>
>><br>
>> Mobile +39 3394823805<br>
>> Skype paskporr<br>
>><br>
>><br>
>> ______________________________<u></u>_________________<br>
>> OpenStack-dev mailing list<br>
>> <a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
>><br>
> ______________________________<u></u>_________________<br>
> OpenStack-dev mailing list<br>
> <a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
<br>
--<br>
Pasquale Porreca<br>
<br>
DEK Technologies<br>
Via dei Castelli Romani, 22<br>
00040 Pomezia (Roma)<br>
<br>
Mobile +39 3394823805<br>
Skype paskporr<br>
<br>
<br>
______________________________<u></u>_________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
</blockquote></div>