Re: [openstack-dev] [python-cinderclient] Return request ID to caller

2015-02-06 Thread Joe Gordon
On Thu, Feb 5, 2015 at 11:24 PM, Kekane, Abhishek 
abhishek.kek...@nttdata.com wrote:

  Hi Devs,



 This change is not backward compatible and to do not break OpenStack
 services which are using cinder-client,

 we need to first make provision in these consumer services to handle
 cinder-client return type change.

 To make this cinder-client change backward compatible we need to do
 changes in consumers of cinder-client like patch :
 https://review.openstack.org/#/c/152820/



 Also for backward compatibility can we make changes suggested by Gary W.
 Smith on cinder-spec : https://review.openstack.org/#/c/132161/6/.

 As per his suggestion we need to add one new optional kwarg
 'return_req_id' in cinder-client api methods, when it is 'True'
 cinder-client will returns the tuple, but when False (the default) it
 returns the current value (i.e.- only response body).



 For example cinder-client 'get' method will look like -



 def _get(self, url, response_key=None, return_req_id=False):

 resp, body = self.api.client.get(url)

 if response_key:

 body = self.resource_class(self, body[response_key],
 loaded=True)

 else:

 body = self.resource_class(self, body, loaded=True)



 if return_req_id:

 # return tuple containing headers and body

 return (resp.headers, body)



 return body





 If we want headers from cinder-client then we need to pass kwarg
 'return_req_id' as True from caller.

 For example from nova we need to call cinder-client get method as -



 resp_header, volume = cinderclient(context).volumes.get(volume_id,
 return_req_id=True)





 With this optional kwarg 'return_req_id' approach we do not need to make
 changes in services which are using cinder-client.

 It will be backward compatible change.


Maintaining backwards compatibility is very important. Making return_req_id
optional sounds like a good solution going forward.




 Could you please give your suggestion on this approach.



 Thanks,



 Abhishek





 *From:* Joe Gordon [mailto:joe.gord...@gmail.com]
 *Sent:* 05 February 2015 22:50
 *To:* OpenStack Development Mailing List (not for usage questions)
 *Subject:* Re: [openstack-dev] [python-cinderclient] Return request ID to
 caller







 On Wed, Feb 4, 2015 at 11:23 PM, Malawade, Abhijeet 
 abhijeet.malaw...@nttdata.com wrote:

 Hi,



 I have submitted patch for cinder-client [1] to 'Return tuple containing
 header and body from client' instead of just response.

 Also cinder spec for the same is under review [2].



 This change will break OpenStack services which are using cinder-client.
 To do not break services which are using cinder-client,

 we need to first make changes in those projects to check return type of
 cinder-client. We are working on doing cinder-client return

 type check changes in OpenStack services like nova, glance_store, heat,
 trove, manila etc.

 We have already submitted patch for same against nova :
 https://review.openstack.org/#/c/152820/



 [1] https://review.openstack.org/#/c/152075/

 [2] https://review.openstack.org/#/c/132161/



 This sounds like a backwards incompatible change to the python client,
 that will break downstream consumers of python-cinderclient. This change
 should be done in a way that allows us to deprecate the old usage without
 breaking it right away.





 I want to seek early feedback from the community members on the above
 patches, so please give your thoughts on the same.



 Thanks,

 Abhijeet Malawade


 __
 Disclaimer: This email and any attachments are sent in strictest confidence
 for the sole use of the addressee and may contain legally privileged,
 confidential, and proprietary data. If you are not the intended recipient,
 please advise the sender by replying promptly to this email and then delete
 and destroy this email and any attachments without any further use, copying
 or forwarding.


 __
 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



 __
 Disclaimer: This email and any attachments are sent in strictest confidence
 for the sole use of the addressee and may contain legally privileged,
 confidential, and proprietary data. If you are not the intended recipient,
 please advise the sender by replying promptly to this email and then delete
 and destroy this email and any attachments without any further use, copying
 or forwarding.

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

Re: [openstack-dev] [python-cinderclient] Return request ID to caller

2015-02-05 Thread Kekane, Abhishek
Hi Devs,

This change is not backward compatible and to do not break OpenStack services 
which are using cinder-client,
we need to first make provision in these consumer services to handle 
cinder-client return type change.
To make this cinder-client change backward compatible we need to do changes in 
consumers of cinder-client like patch : https://review.openstack.org/#/c/152820/

Also for backward compatibility can we make changes suggested by Gary W. Smith 
on cinder-spec : https://review.openstack.org/#/c/132161/6/.
As per his suggestion we need to add one new optional kwarg 'return_req_id' in 
cinder-client api methods, when it is 'True' cinder-client will returns the 
tuple, but when False (the default) it returns the current value (i.e.- only 
response body).

For example cinder-client 'get' method will look like -

def _get(self, url, response_key=None, return_req_id=False):
resp, body = self.api.client.get(url)
if response_key:
body = self.resource_class(self, body[response_key], loaded=True)
else:
body = self.resource_class(self, body, loaded=True)

if return_req_id:
# return tuple containing headers and body
return (resp.headers, body)

return body


If we want headers from cinder-client then we need to pass kwarg 
'return_req_id' as True from caller.
For example from nova we need to call cinder-client get method as -

resp_header, volume = cinderclient(context).volumes.get(volume_id, 
return_req_id=True)


With this optional kwarg 'return_req_id' approach we do not need to make 
changes in services which are using cinder-client.
It will be backward compatible change.

Could you please give your suggestion on this approach.

Thanks,

Abhishek


From: Joe Gordon [mailto:joe.gord...@gmail.com]
Sent: 05 February 2015 22:50
To: OpenStack Development Mailing List (not for usage questions)
Subject: Re: [openstack-dev] [python-cinderclient] Return request ID to caller



On Wed, Feb 4, 2015 at 11:23 PM, Malawade, Abhijeet 
abhijeet.malaw...@nttdata.commailto:abhijeet.malaw...@nttdata.com wrote:
Hi,

I have submitted patch for cinder-client [1] to 'Return tuple containing header 
and body from client' instead of just response.
Also cinder spec for the same is under review [2].

This change will break OpenStack services which are using cinder-client. To do 
not break services which are using cinder-client,
we need to first make changes in those projects to check return type of 
cinder-client. We are working on doing cinder-client return
type check changes in OpenStack services like nova, glance_store, heat, trove, 
manila etc.
We have already submitted patch for same against nova : 
https://review.openstack.org/#/c/152820/

[1] https://review.openstack.org/#/c/152075/
[2] https://review.openstack.org/#/c/132161/

This sounds like a backwards incompatible change to the python client, that 
will break downstream consumers of python-cinderclient. This change should be 
done in a way that allows us to deprecate the old usage without breaking it 
right away.


I want to seek early feedback from the community members on the above patches, 
so please give your thoughts on the same.

Thanks,
Abhijeet Malawade

__
Disclaimer: This email and any attachments are sent in strictest confidence
for the sole use of the addressee and may contain legally privileged,
confidential, and proprietary data. If you are not the intended recipient,
please advise the sender by replying promptly to this email and then delete
and destroy this email and any attachments without any further use, copying
or forwarding.

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


__
Disclaimer: This email and any attachments are sent in strictest confidence
for the sole use of the addressee and may contain legally privileged,
confidential, and proprietary data. If you are not the intended recipient,
please advise the sender by replying promptly to this email and then delete
and destroy this email and any attachments without any further use, copying
or forwarding.
__
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] [python-cinderclient] Return request ID to caller

2015-02-05 Thread Joe Gordon
On Wed, Feb 4, 2015 at 11:23 PM, Malawade, Abhijeet 
abhijeet.malaw...@nttdata.com wrote:

  Hi,



 I have submitted patch for cinder-client [1] to 'Return tuple containing
 header and body from client' instead of just response.

 Also cinder spec for the same is under review [2].



 This change will break OpenStack services which are using cinder-client.
 To do not break services which are using cinder-client,

 we need to first make changes in those projects to check return type of
 cinder-client. We are working on doing cinder-client return

 type check changes in OpenStack services like nova, glance_store, heat,
 trove, manila etc.

 We have already submitted patch for same against nova :
 https://review.openstack.org/#/c/152820/



 [1] https://review.openstack.org/#/c/152075/

 [2] https://review.openstack.org/#/c/132161/


This sounds like a backwards incompatible change to the python client, that
will break downstream consumers of python-cinderclient. This change should
be done in a way that allows us to deprecate the old usage without breaking
it right away.




 I want to seek early feedback from the community members on the above
 patches, so please give your thoughts on the same.



 Thanks,

 Abhijeet Malawade

 __
 Disclaimer: This email and any attachments are sent in strictest confidence
 for the sole use of the addressee and may contain legally privileged,
 confidential, and proprietary data. If you are not the intended recipient,
 please advise the sender by replying promptly to this email and then delete
 and destroy this email and any attachments without any further use, copying
 or forwarding.

 __
 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-dev] [python-cinderclient] Return request ID to caller

2015-02-04 Thread Malawade, Abhijeet
Hi,

I have submitted patch for cinder-client [1] to 'Return tuple containing header 
and body from client' instead of just response.
Also cinder spec for the same is under review [2].

This change will break OpenStack services which are using cinder-client. To do 
not break services which are using cinder-client,
we need to first make changes in those projects to check return type of 
cinder-client. We are working on doing cinder-client return
type check changes in OpenStack services like nova, glance_store, heat, trove, 
manila etc.
We have already submitted patch for same against nova : 
https://review.openstack.org/#/c/152820/

[1] https://review.openstack.org/#/c/152075/
[2] https://review.openstack.org/#/c/132161/

I want to seek early feedback from the community members on the above patches, 
so please give your thoughts on the same.

Thanks,
Abhijeet Malawade

__
Disclaimer: This email and any attachments are sent in strictest confidence
for the sole use of the addressee and may contain legally privileged,
confidential, and proprietary data. If you are not the intended recipient,
please advise the sender by replying promptly to this email and then delete
and destroy this email and any attachments without any further use, copying
or forwarding.__
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] [python-cinderclient] Return request ID to caller

2014-12-17 Thread Mike Perez
On 05:54 Fri 12 Dec , Malawade, Abhijeet wrote:
 HI,
 
 I want your thoughts on blueprint 'Log Request ID Mappings' for cross 
 projects.
 BP: https://blueprints.launchpad.net/nova/+spec/log-request-id-mappings
 It will enable operators to get request id's mappings easily and will be 
 useful in analysing logs effectively.

I've weighed on this question a couple of times now and recently from the
Cinder meeting. Solution 1 please.

-- 
Mike Perez

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


Re: [openstack-dev] [python-cinderclient] Return request ID to caller

2014-12-17 Thread Jamie Lennox


- Original Message -
 From: Abhijeet Malawade abhijeet.malaw...@nttdata.com
 To: openstack-dev@lists.openstack.org
 Sent: Friday, 12 December, 2014 3:54:04 PM
 Subject: [openstack-dev] [python-cinderclient] Return request ID to caller
 
 
 
 HI,
 
 
 
 I want your thoughts on blueprint 'Log Request ID Mappings’ for cross
 projects.
 
 BP: https://blueprints.launchpad.net/nova/+spec/log-request-id-mappings
 
 It will enable operators to get request id's mappings easily and will be
 useful in analysing logs effectively.
 
 
 
 For logging 'Request ID Mappings', client needs to return
 'x-openstack-request-id' to the caller.
 
 Currently python-cinderclient do not return 'x-openstack-request-id' back to
 the caller.
 
 
 
 As of now, I could think of below two solutions to return 'request-id' back
 from cinder-client to the caller.
 
 
 
 1. Return tuple containing response header and response body from all
 cinder-client methods.
 
 (response header contains 'x-openstack-request-id').
 
 
 
 Advantages:
 
 A. In future, if the response headers are modified then it will be available
 to the caller without making any changes to the python-cinderclient code.
 
 
 
 Disadvantages:
 
 A. Affects all services using python-cinderclient library as the return type
 of each method is changed to tuple.
 
 B. Need to refactor all methods exposed by the python-cinderclient library.
 Also requires changes in the cross projects wherever python-cinderclient
 calls are being made.
 
 
 
 Ex. :-
 
 From Nova, you will need to call cinder-client 'get' method like below :-
 
 resp_header, volume = cinderclient(context).volumes.get(volume_id)
 
 
 
 x-openstack-request-id = resp_header.get('x-openstack-request-id', None)
 
 
 
 Here cinder-client will return both response header and volume. From response
 header, you can get 'x-openstack-request-id'.
 
 
 
 2. The optional parameter 'return_req_id' of type list will be passed to each
 of the cinder-client method. If this parameter is passed then cinder-client
 will append ‘'x-openstack-request-id' received from cinder api to this list.
 
 
 
 This is already implemented in glance-client (for V1 api only)
 
 Blueprint :
 https://blueprints.launchpad.net/python-glanceclient/+spec/return-req-id
 
 Review link : https://review.openstack.org/#/c/68524/7
 
 
 
 Advantages:
 
 A. Requires changes in the cross projects only at places wherever
 python-cinderclient calls are being made requiring 'x-openstack-request-id’.
 
 
 
 Dis-advantages:
 
 A. Need to refactor all methods exposed by the python-cinderclient library.
 
 
 
 Ex. :-
 
 From Nova, you will need to pass return_req_id parameter as a list.
 
 kwargs['return_req_id'] = []
 
 item = cinderclient(context).volumes.get(volume_id, **kwargs)
 
 
 
 if kwargs.get('return_req_id'):
 
 x-openstack-request-id = kwargs['return_req_id'].pop()
 
 
 
 python-cinderclient will add 'x-openstack-request-id' to the 'return_req_id'
 list if it is provided in kwargs.
 
 
 
 IMO, solution #2 is better than #1 for the reasons quoted above.
 
 Takashi NATSUME has already proposed a patch for solution #2. Please review
 patch https://review.openstack.org/#/c/104482/.
 
 Would appreciate if you can think of any other better solution than #2.
 
 
 
 Thank you.
 
 

Abhijeet

So option 1 is a massive compatibility break. There's no way you can pull of a 
change in the return value like that without a new major version and every 
getting annoyed. 

My question is why does it need to be returned to the caller? What is the 
caller going to do with it other than send it to the debug log? It's an admin 
who is trying to figure out those logs later that wants the request-id included 
in that information, not the application at run time. 

Why not just have cinderclient log it as part of the standard request logging: 
https://github.com/openstack/python-cinderclient/blob/master/cinderclient/client.py#L170



Jamie

 __
 Disclaimer: This email and any attachments are sent in strictest confidence
 for the sole use of the addressee and may contain legally privileged,
 confidential, and proprietary data. If you are not the intended recipient,
 please advise the sender by replying promptly to this email and then delete
 and destroy this email and any attachments without any further use, copying
 or forwarding.
 
 ___
 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] [python-cinderclient] Return request ID to caller

2014-12-11 Thread Malawade, Abhijeet
HI,

I want your thoughts on blueprint 'Log Request ID Mappings' for cross projects.
BP: https://blueprints.launchpad.net/nova/+spec/log-request-id-mappings
It will enable operators to get request id's mappings easily and will be useful 
in analysing logs effectively.

For logging 'Request ID Mappings', client needs to return 
'x-openstack-request-id' to the caller.
Currently python-cinderclient do not return 'x-openstack-request-id' back to 
the caller.

As of now, I could think of below two solutions to return 'request-id' back 
from cinder-client to the caller.

1. Return tuple containing response header and response body from all 
cinder-client methods.
  (response header contains 'x-openstack-request-id').

Advantages:

A.  In future, if the response headers are modified then it will be 
available to the caller without making any changes to the python-cinderclient 
code.


Disadvantages:
A. Affects all services using python-cinderclient library as the  return 
type of each method is changed to tuple.
 B. Need to refactor all methods exposed by the python-cinderclient 
library. Also requires changes in the cross projects wherever 
python-cinderclient calls are being made.

Ex. :-
 From Nova, you will need to call cinder-client 'get' method like below  :-
   resp_header, volume = cinderclient(context).volumes.get(volume_id)

x-openstack-request-id = resp_header.get('x-openstack-request-id', None)

Here cinder-client will return both response header and volume. From response 
header, you can get 'x-openstack-request-id'.

2. The optional parameter 'return_req_id' of type list will be passed to each 
of the cinder-client method. If this parameter is passed then cinder-client 
will append ''x-openstack-request-id' received from cinder api to this list.

This is already implemented in glance-client (for V1 api only)
Blueprint : 
https://blueprints.launchpad.net/python-glanceclient/+spec/return-req-id
Review link : https://review.openstack.org/#/c/68524/7

Advantages:

A.  Requires changes in the cross projects only at places wherever 
python-cinderclient calls are being made requiring 'x-openstack-request-id'.


Dis-advantages:

A.  Need to refactor all methods exposed by the python-cinderclient library.


Ex. :-
From Nova, you will need to pass  return_req_id parameter as a list.
kwargs['return_req_id'] = []
item = cinderclient(context).volumes.get(volume_id, **kwargs)

if kwargs.get('return_req_id'):
x-openstack-request-id = kwargs['return_req_id'].pop()

python-cinderclient will add 'x-openstack-request-id' to the 'return_req_id' 
list if it is provided in kwargs.

IMO, solution #2 is better than #1 for the reasons quoted above.
Takashi NATSUME has already proposed a patch for solution #2.  Please review 
patch https://review.openstack.org/#/c/104482/.
Would appreciate if you can think of any other better solution than #2.

Thank you.
Abhijeet

__
Disclaimer: This email and any attachments are sent in strictest confidence
for the sole use of the addressee and may contain legally privileged,
confidential, and proprietary data. If you are not the intended recipient,
please advise the sender by replying promptly to this email and then delete
and destroy this email and any attachments without any further use, copying
or forwarding.___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev