jimbobhickville / python-ambariclient

Python client bindings for the Apache Ambari REST API
44 stars 31 forks source link

Getting content from request #28

Closed dimaspivak closed 7 years ago

dimaspivak commented 7 years ago

Hi @jimbobhickville,

I have one more question for an enhancement I'd like to make to your project (if it's alright with you). I have a use case that requires me to get a tarstream of the client configs for a given service from Ambari. This is straightforward enough to implement with a new ClusterServiceComponentCollection class with a get_client_config_tar method, but I'm having trouble deciding how to make it fit into your existing semantics. In short, the self.loads(self.client.get(...)) implementation pattern used throughout fails for me because the self.client.get part ends up returning a response.json() which can't be parsed with the JSON encoder (i.e. because the response we're getting isn't a JSON but a tarstream).

A working example I implemented drops abstractions to the wayside and simply uses the underlying requests machinery, but I wanted to see if you had a thought on if there's a cleaner way to architect this:

class ClusterServiceComponentCollection(base.QueryableModelCollection):
    def get_client_config_tar(self):
        url = self.url + "?format=client_config_tar"
        http_client = self.client.client
        return http_client.session.get(url, **http_client.request_params)
jimbobhickville commented 7 years ago

I originally wrote it to only automatically json-decode on the proper content-type, but Ambari doesn't always return the right content-type. It returns text/plain for json in a lot of places, for example, so since everything was JSON at the time, I just made that the default path.

https://github.com/jimbobhickville/python-ambariclient/blob/master/ambariclient/client.py#L161

Some other options besides the proposed (which looks like it would also work, but a more generic solution might be useful if this sort of thing comes up again):

A) add an optional parameter to that method to tell it you aren't expecting a json response, so self.client.get(..., is_json=False) or something B) modify that method to handle other known content-types and fallback to json-decoding if it gets one that it doesn't know what to do with. Then just return the raw response if it has the tar content-type (I'd have to look it up to see what that is).

dimaspivak commented 7 years ago

Done and merged. Thanks again for the super fast turnaround, @jimbobhickville!