Re: [openstack-dev] [neutron][api] advanced search criteria

[Hirofumi Ichihara]

On 2016/04/05 22:23, Ihar Hrachyshka wrote:

Hirofumi Ichihara  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, …).
The patch is reasonable for me as first step. Second, I agree to make it 
more generic. I think that we should have test per resource but we will 
do in future work.




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.

+1

Thanks,
Hirofumi



Ihar

__ 


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: 
openstack-dev-requ...@lists.openstack.org?subject:unsubscribe

http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev





__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Kyle Mestery]

On Mon, Apr 4, 2016 at 7:36 PM, Armando M.  wrote:
>
>
> On 4 April 2016 at 17:08, Jay Pipes  wrote:
>>
>> On 04/04/2016 06:57 PM, Ihar Hrachyshka wrote:
>>>
>>> - why do we even need to control those features with configuration
>>> options? can we deprecate and remove them?
>>
>>
>> This would be my choice. Configuration options that make the Neutron API
>> behave differently from one deployment to another should be put out to
>> pasture.
>
>
> So long as we figure out a way to provide support these capabilities
> natively, I agree that we should stop using config options to alter API
> behavior. This is undiscoverable by the end user, who is left with the only
> choice of poking at the API to see how it responds.
>
Which is at best an awful user experience.

So +1 to removing them.

Kyle

> Cheers,
> Armando
>
>>
>>
>> Best,
>> -jay
>>
>>
>> __
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Ihar Hrachyshka]

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

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Ihar Hrachyshka]

Armando M.  wrote:




On 4 April 2016 at 17:08, Jay Pipes  wrote:
On 04/04/2016 06:57 PM, Ihar Hrachyshka wrote:
- why do we even need to control those features with configuration
options? can we deprecate and remove them?

This would be my choice. Configuration options that make the Neutron API  
behave differently from one deployment to another should be put out to  
pasture.


So long as we figure out a way to provide support these capabilities  
natively, I agree that we should stop using config options to alter API  
behavior. This is undiscoverable by the end user, who is left with the  
only choice of poking at the API to see how it responds.


We already provide a way for plugins to claim native support for those  
features, and if they don’t claim it, we fall back to generic  
implementation in Python. Isn’t it enough?


Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Hirofumi Ichihara]

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.



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

Thanks,
Hirofumi



Ihar

__ 


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: 
openstack-dev-requ...@lists.openstack.org?subject:unsubscribe

http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev





__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Armando M.]

On 4 April 2016 at 17:08, Jay Pipes  wrote:

> On 04/04/2016 06:57 PM, Ihar Hrachyshka wrote:
>
>> - why do we even need to control those features with configuration
>> options? can we deprecate and remove them?
>>
>
> This would be my choice. Configuration options that make the Neutron API
> behave differently from one deployment to another should be put out to
> pasture.
>

So long as we figure out a way to provide support these capabilities
natively, I agree that we should stop using config options to alter API
behavior. This is undiscoverable by the end user, who is left with the only
choice of poking at the API to see how it responds.

Cheers,
Armando


>
> Best,
> -jay
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron][api] advanced search criteria

[Jay Pipes]

On 04/04/2016 06:57 PM, Ihar Hrachyshka wrote:

- why do we even need to control those features with configuration
options? can we deprecate and remove them?


This would be my choice. Configuration options that make the Neutron API 
behave differently from one deployment to another should be put out to 
pasture.


Best,
-jay

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [neutron][api] advanced search criteria

[Ihar Hrachyshka]

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.


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?


Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev