Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
Hi, I'd rather not subclass dict directly. for various reasons adding extra attributes to normal python dict seems prone to errors since people will be expecting regular dicts, and on the other hand if we want to expand it in the future we might run into problems playing with dict methods (such as update) I suggets (roughly): class ResponseBody(dict): def __init__(self, body=None, resp=None): self_data_dict = body or {} self.resp = resp def __getitem__(self, index): return self._data_dict[index] Thus we can keep the previous dict interface, but protect the data and make sure the object will behave exactly as we expect it to. if we want it to have more dict attributes/method we can add them explicitly - Original Message - From: Boris Pavlovic bpavlo...@mirantis.com To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org Sent: Saturday, August 30, 2014 2:53:37 PM Subject: Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients Sean, class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Are you sure that you would like to have default value {} for method argument and not something like: class ResponseBody(dict): def __init__(self, body=None, resp=None): body = body or {} self.update(body) self.resp = resp In your case you have side effect. Take a look at: http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument Best regards, Boris Pavlovic On Sat, Aug 30, 2014 at 10:08 AM, GHANSHYAM MANN ghanshyamm...@gmail.com wrote: +1. That will also help full for API coming up with microversion like Nova. On Fri, Aug 29, 2014 at 11:56 PM, Sean Dague s...@dague.net wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Thanks Regards Ghanshyam Mann ___ 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
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
+1 keeping the body as a ~dict will help with all existing asserts comparing dicts in tests. Andrea On 30 Aug 2014 06:45, Christopher Yeoh cbky...@gmail.com wrote: On Fri, 29 Aug 2014 11:13:39 -0400 David Kranz dkr...@redhat.com wrote: On 08/29/2014 10:56 AM, Sean Dague wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean Heh. I agree with that and it is along a similar line to what I proposed here https://review.openstack.org/#/c/106916/ but using a dict rather than an attribute dict. I did not propose this since it is such a big change. All the test code would have to be changed to remove the resp or _ that is now receiving the response. But I think we should do this before the client code is moved to tempest-lib. +1. this would be a nice cleanup. Chris ___ 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] [qa] Lack of consistency in returning response from tempest clients
+1. That will also help full for API coming up with microversion like Nova. On Fri, Aug 29, 2014 at 11:56 PM, Sean Dague s...@dague.net wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Thanks Regards Ghanshyam Mann ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
Sean, class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Are you sure that you would like to have default value {} for method argument and not something like: class ResponseBody(dict): def __init__(self, body=None, resp=None): body = body or {} self.update(body) self.resp = resp In your case you have side effect. Take a look at: http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument Best regards, Boris Pavlovic On Sat, Aug 30, 2014 at 10:08 AM, GHANSHYAM MANN ghanshyamm...@gmail.com wrote: +1. That will also help full for API coming up with microversion like Nova. On Fri, Aug 29, 2014 at 11:56 PM, Sean Dague s...@dague.net wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Thanks Regards Ghanshyam Mann ___ 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] [qa] Lack of consistency in returning response from tempest clients
While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. -David ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Sounds good to me. :) -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean -- Sean Dague http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
On 08/29/2014 10:56 AM, Sean Dague wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean Heh. I agree with that and it is along a similar line to what I proposed here https://review.openstack.org/#/c/106916/ but using a dict rather than an attribute dict. I did not propose this since it is such a big change. All the test code would have to be changed to remove the resp or _ that is now receiving the response. But I think we should do this before the client code is moved to tempest-lib. -David ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [qa] Lack of consistency in returning response from tempest clients
Yeah, Sean's proposal looks great to me On Fri, Aug 29, 2014 at 10:13 AM, David Kranz dkr...@redhat.com wrote: On 08/29/2014 10:56 AM, Sean Dague wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean Heh. I agree with that and it is along a similar line to what I proposed here https://review.openstack.org/#/c/106916/ but using a dict rather than an attribute dict. I did not propose this since it is such a big change. All the test code would have to be changed to remove the resp or _ that is now receiving the response. But I think we should do this before the client code is moved to tempest-lib. -David ___ 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] [qa] Lack of consistency in returning response from tempest clients
On Fri, 29 Aug 2014 11:13:39 -0400 David Kranz dkr...@redhat.com wrote: On 08/29/2014 10:56 AM, Sean Dague wrote: On 08/29/2014 10:19 AM, David Kranz wrote: While reviewing patches for moving response checking to the clients, I noticed that there are places where client methods do not return any value. This is usually, but not always, a delete method. IMO, every rest client method should return at least the response. Some services return just the response for delete methods and others return (resp, body). Does any one object to cleaning this up by just making all client methods return resp, body? This is mostly a change to the clients. There were only a few places where a non-delete method was returning just a body that was used in test code. Yair and I were discussing this yesterday. As the response correctness checking is happening deeper in the code (and you are seeing more and more people assigning the response object to _ ) my feeling is Tempest clients should probably return a body obj that's basically. class ResponseBody(dict): def __init__(self, body={}, resp=None): self.update(body) self.resp = resp Then all the clients would have single return values, the body would be the default thing you were accessing (which is usually what you want), and the response object is accessible if needed to examine headers. -Sean Heh. I agree with that and it is along a similar line to what I proposed here https://review.openstack.org/#/c/106916/ but using a dict rather than an attribute dict. I did not propose this since it is such a big change. All the test code would have to be changed to remove the resp or _ that is now receiving the response. But I think we should do this before the client code is moved to tempest-lib. +1. this would be a nice cleanup. Chris ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev