Re: [openstack-dev] [Trove] Datastore/Versions API improvements

2014-08-05 Thread Craig Vyvial
On Wed, Jul 30, 2014 at 10:10 AM, Denis Makogon dmako...@mirantis.com
wrote:

 Hello, Stackers.



 I’d like to gather Trove team around question related to
 Datastores/Version API responses (request/response payloads and HTTP codes).

 Small INFO

 When deployer creates datastore and versions for it Troves` backend
 receives request to store DBDatastore and DBDatastoreVersion objects with
 certain parameters. The most interesting attribute of DBDatastoreVersion is
 “packages” - it’s being stored as String object (and it’s totally fine).
 But when we’re trying to query given datastore version through the
 Datastores API attribute “packages” is being returned as String object too.
 And it seems that it breaks response pattern - “If given attribute
 represents complex attribute, such as: list, dict, tuple - it should be
 returned as is.

 So, the first question is - are we able to change it in terms of V1?

If it does not break the public api then i do not think there is an issue
making a change.
I made a change not long ago around making the packages a list thats sent
to the guest. I'm a bit confused what you are wanting to change here.
Are you suggesting changing the data that is stored for packages (string to
a json.dumps list or something).
Or making the model parse the string into a list when you request the
packages for a datastore version?



 The second question is about admin_context decorator (see [1]). This
 method executes methods of given controller and verifies that user is able
 to execute certain procedure.

 Taking into account RFC 2616 this method should raise HTTP Forbidden
 (code 403) if user tried to execute request that he’s not allowed to.

 But given method return HTTP Unauthorized (code 401) which seems
 weird since user is authorized.

I think this is a valid bug for the error code although the message makes
it clear why you get the 401.
https://github.com/openstack/trove/blob/master/trove/common/auth.py#L85



 This is definitely a bug. And it comes from [2].


 [1]
 https://github.com/openstack/trove/blob/master/trove/common/auth.py#L72-L87

 [2]
 https://github.com/openstack/trove/blob/master/trove/common/wsgi.py#L316-L318



 Best regards,

 Denis Makogon

 ___
 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


Re: [openstack-dev] [Trove] Datastore/Versions API improvements

2014-08-05 Thread Denis Makogon
On Tue, Aug 5, 2014 at 11:06 PM, Craig Vyvial cp16...@gmail.com wrote:




 On Wed, Jul 30, 2014 at 10:10 AM, Denis Makogon dmako...@mirantis.com
 wrote:

 Hello, Stackers.



 I’d like to gather Trove team around question related to
 Datastores/Version API responses (request/response payloads and HTTP codes).

 Small INFO

 When deployer creates datastore and versions for it Troves` backend
 receives request to store DBDatastore and DBDatastoreVersion objects with
 certain parameters. The most interesting attribute of DBDatastoreVersion is
 “packages” - it’s being stored as String object (and it’s totally fine).
 But when we’re trying to query given datastore version through the
 Datastores API attribute “packages” is being returned as String object too.
 And it seems that it breaks response pattern - “If given attribute
 represents complex attribute, such as: list, dict, tuple - it should be
 returned as is.

 So, the first question is - are we able to change it in terms of V1?

 If it does not break the public api then i do not think there is an issue
 making a change.


If modification means breaking then yes. I would say that type 'packages'
attribute should be changed to more appropriate type, such as list of
string. But it seems that this modification would be possible in abstract
V2,


 I made a change not long ago around making the packages a list thats sent
 to the guest. I'm a bit confused what you are wanting to change here.
 Are you suggesting changing the data that is stored for packages (string
 to a json.dumps list or something).
 Or making the model parse the string into a list when you request the
 packages for a datastore version?

I guess last thing. If i want to iterate over packages i would need to
manually split string an build appropriate data type.




 The second question is about admin_context decorator (see [1]). This
 method executes methods of given controller and verifies that user is able
 to execute certain procedure.

 Taking into account RFC 2616 this method should raise HTTP Forbidden
 (code 403) if user tried to execute request that he’s not allowed to.

 But given method return HTTP Unauthorized (code 401) which seems
 weird since user is authorized.

 I think this is a valid bug for the error code although the message makes
 it clear why you get the 401.
 https://github.com/openstack/trove/blob/master/trove/common/auth.py#L85


The problem is that user is authorized but doesn't have certain
permissions. Unauthorized means that user passed wrong credentials,
Forbidden (in terms of ReST) authorized but not permitted.

Craig, after digging into the problem i found out where current code is
broken, see
https://github.com/openstack/trove/blob/master/trove/common/wsgi.py#L316-L318




 This is definitely a bug. And it comes from [2].


 [1]
 https://github.com/openstack/trove/blob/master/trove/common/auth.py#L72-L87

 [2]
 https://github.com/openstack/trove/blob/master/trove/common/wsgi.py#L316-L318



 Best regards,

 Denis Makogon

 ___
 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


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev