Open Stack

Hirofumi Ichihara <ichihara.hirofumi at lab.ntt.co.jp> wrote:

> Hi Ihar,
>
> On 2016/04/05 7:57, Ihar Hrachyshka wrote:
>> Hi all,
>>
>> in neutron, we have a bunch of configuration options to control advanced  
>> filtering features for API, f.e. allow_sorting, allow_pagination,  
>> allow_bulk, etc. Those options have default False values.
> I saw allow_bulk option is set default True in  
> https://github.com/openstack/neutron/blob/master/neutron/common/config.py#L66
> Well, I don't think there's someone sets False to the option.

Yes, indeed only allow_sorting and allow_pagination are disabled by default.

>
>> In the base API controller class, we have support for both native  
>> sorting/pagination/bulk operations [implemented by the plugin itself],  
>> as well as a generic implementation for plugins without native support.  
>> But if corresponding allow_* options are left with their default False  
>> values, those advanced search/filtering criteria just don’t work, no  
>> matter whether the plugin support native filters, or not.
>>
>> It seems weird to me that our API behaves differently depending on  
>> configuration options, and that we have those useful features disabled  
>> by default.
>>
>> My immediate interest is to add native support for sorting/pagination  
>> for QoS service plugin; I have a patch for that, and I planned to add  
>> some API tests to validate that the features work, but I hit failures  
>> because those features are not enabled for the -api job.
>>
>> Some questions:
>> - can we enable those features in -api job?
>> - is there any reason to keep default values for allow_* as False, and  
>> if not, can we switch to True?
>> - why do we even need to control those features with configuration  
>> options? can we deprecate and remove them?
> I agree we will deprecate and remove the option but I think that we need  
> more tests if we support it as default.
> It looks like there are very few tests(UT only).

That’s a good suggestion. I started a patch to enable those two options,  
plus add first tests for the feature:

https://review.openstack.org/#/c/301634/

For now it covers only for networks. I wonder how we envision the coverage.  
Do we want to have test cases per resource? Any ideas on how to make the  
code more generic to avoid code duplication? For example, I could move  
those test cases into a base class that would require some specialization  
for each resource that we want to cover (get/create methods, primary key,  
…).

Also, do we maybe want to split the patch into two pieces:
- first one adding tests [plus enabling those features for API job];
- second one changing the default value for the options.

Ihar