Refactor endpoints

[pocin]

1. I refactored the base Endpoint get, put, delete methods to "private" (_get, _put, _post)
   This is because there were some naming collisions in Buckets.delete() and the super() was being abused.

---

2. Next I refactored the authentication header handling to the base Endpoint class. This new style still allows passing of custom headers (exactly the same way as you'd expect).

class Tables(Endpoint):
    ...
    def list(self):
        custom_headers = {'X-custom-header1': "12345"}
        self._get(urljoin(self.base_url, 'list'), headers=custom_headers)

In the example above, the request headers will contain both X-custom-header1 and X-storage-token. In case of POST requests, the content-type: application/x-www-form-urlencoded is added automatically (more on this in #25 soon)

---

3. Lastly (and most importantly) I introduced a as_json argument to Endpoint._get method.
   This is because the Tables.preview() endpoint doesn't return json. The default behavior is still to return json, though.

Now I am thinking we might try something like this (which I favor the least):

#in the Endpoint class
class Endpoint:
    def _get(self, url, params, **kwargs):
        r = requests.get(url, params, **kwargs)
        try:
            r.raise_for_status()
        except requests.HTTPError:
            # Handle different error codes
            raise
        else:
            try:
                return r.json()
            except JSONDecodeError:
                return r.content.decode('utf-8')

Or maybe we can always return the Response object and let the child endpoint methods handle the decoding?

class Endpoint:
    def _get(self, url, params, **kwargs):
        r = requests.get(url, params, **kwargs)
        try:
            r.raise_for_status()
        except requests.HTTPError:
            # Handle different error codes
            raise
        else:
            return r

class Tables(Endpoint):
    def list(self):
        resp = self._get(urljoin(self.base_url, 'list'))
        # it also allows you to access resp.headers (if they are ever useful, idk)
        return resp.json() # or resp.content.decode('utf-8')

This last one seems pretty flexible and now when I am thinking about it I like it the most. What do you think?

Travis was passing, so I think it should be OK.

[odinuv]

I dislike the as_json=False having two methods would make more sense (either _get _get_json or _get and get_raw) - one would call the other

[Ogaday]

Taken me a while to get to this sorry! Was on holiday and when I got back by bathroom ceiling had collapsed due to a leak upstairs...

1. I refactored the base Endpoint get, put, delete methods

Fine by me.

2. Next I refactored the authentication header handling to the base Endpoint class.

I'd be happy to wait a bit longer be refactoring this but this is fine ^^

I dislike the as_json=False having two methods would make more sense

I agree with this. Also think that returning the response itself could work, but I think 95% of the time you just want to the json response, so handling the response each time would lead to a lot of duplication.

[pocin]

Yeah, the as_json argument is stupid.

I am wondering whether this

class Endpoint:
     def _get(self, url, params, **kwargs):
            r = requests.get(url, params, **kwargs)
            try:
                r.raise_for_status()
            except requests.HTTPError:
                # Handle different error codes
                raise
            else:
                return r

    def _get_json(self, *args, **kwargs):
        return self._get(*args,**kwargs).json()

brings any real benefit. It's more or less the same amount of characters typed and you have one more method to worry about.

I think returning the Response object and handling the serialization in the Endpoint child classes is optimal behavior (isn't there also a case when we have to parse the response headers? I might be confusing this with some other API, but I think some response headers contained the retry-after header )

[odinuv]

if you are talking about this http://docs.keboola.apiary.io/#introduction/http-response-codes then i'd imagine it would be handled in the _get method, this is common behavior for all endpoints

[pocin]

I'm dropping this PR as this one is close to impossible to merge (probably due to the unification of line endings from dos to unix + since there are some new commits in the master.
I created a new one instead (#36) with the same commits as in this one, but squashed into one big commit (again the dos/unix endings made issues) and rebased onto current master.
It's confusing, but I saw no other way around.

Now I see it was too early for such refactoring, lesson learned!

[odinuv]

I still think that something like this

class Endpoint:
     def _get(self, url, params, **kwargs):
            r = requests.get(url, params, **kwargs)
            try:
                r.raise_for_status()
            except requests.HTTPError:
                # Handle different error codes
                raise
            else:
                return r

    def _get_json(self, *args, **kwargs):
        return self._get(*args,**kwargs).json()

is far better than having .json in every request

[Ogaday]

Sorry about the merge conflict ><