5
\$\begingroup\$

I'm creating a Python API client, after thinking and checking some open source Python clients I created this class, and I'm looking for any feedback (design, best practices...), this is an example URL of the API:

https://www.api.com/collection/resource.json?userid=id&key=apikey&command=value

Here's my class using the requests library:

import requests

class APIClient(object):
    """Creates an API client object

    :param userid: the API userid found in the control panel
    :param api_key: the API key found in the control panel Settings > API

    """
    def __init__(self, userid=None, api_key=None):
        # userid and api key sent with every request
        self.userid = userid
        self.key = api_key

        # the base production api url
        self.base_url = None

        # the sandbox api url
        self.test_url = None

        # boolean value to indicate which url to use
        self.testing = False

        # the error description if it occured
        self.error = None

    def request(self, method='GET', path=None, params=None):
        """Makes a request to the API with the given parameters

        :param method: the HTTP method to use
        :param path: the api path to request
        :param params: the parameters to be sent

        """
        # if we're testing use the sandbox url
        api_url = self.test_url if self.testing else self.base_url

        # add the authentication parameters (sent with every request)
        params.update({
            'userid': self.userid,
            'key': self.key
        })

        # the api request result
        result = None

        try:
            # send a request to the api server
            r = requests.request(
                    method = method,
                    url = api_url + path + '.json',
                    params = params,
                    headers = {'User-Agent': 'Python API Client'}
                )

            # raise an exception if status code is not 200
            if r.status_code is not 200:
                raise Exception
            else:
                result = r.json()
        except requests.ConnectionError:
            self.error = 'API connection error.'
        except requests.HTTPError:
            self.error = 'An HTTP error occurred.'
        except requests.Timeout:
            self.error = 'Request timed out.'
        except Exception:
            self.error = 'An unexpected error occurred.'

        return result

And here's a usage example:

from api import APIClient

# create the client
api = APIClient()

# authentication
api.userid = '123456'
api.key = '0tKk8fAyHTlUv'

# api urls
api.base_url = 'https://api.com/'
api.test_url = 'https://test.api.com/'

# api in testing mode
api.testing = True

# create a request
r = api.request(
            method = 'GET',
            # this will be appended to the base or test url and add .json at the end
            path = 'collection/resource',
            params = {
                'string-command': 'string',
                'list-command': ['a', 'b', 'c'],
                'boolean-command': False
            }
        )

# how can I improve the way I get the API result and the error checking ?
# I currently use:
if api.error:
    print api.error
else:
    print r

# what about using something like ?:
if api.request.ok:
    print api.request.result
else:
    print api.request.error

My questions are:

  • Is this a good design for use in a website, and is it safe for multiple requests?
  • Should I store the API result in a property api.result instead of returning it when calling api.request()?
  • How can I improve the way I get the API result? Is the second example better?
  • How can I improve my code?
\$\endgroup\$
8
\$\begingroup\$
  1. Python has a mechanism for handling exceptional situations, but you go to great lengths to suppress it: you catch all the exceptions that might result from your request method and replace them with a string in the .error property of the APIClient object.

    This has several problems.

    First, it needlessly complicates every API call. A caller can't just write:

    r = api.request(...) # might raise an exception
    do_something_with(r)
    

    allowing exceptions to pass up the call stack and so eventually appear on the console or in the log. Instead, they have to write:

    api.error = None # clear the old error, if any
    r = api.request(...) # might set api.error
    if api.error:
        # handle the error somehow
    else:
        do_something_with(r)
    

    This means that every call to your API needs to consider how to handle errors. What are programmers going to do? Well, mostly likely they will raise these errors:

    api.error = None # clear the old error, if any
    r = api.request(...) # might set api.error
    if api.error:
        raise MyError("API error: {}".format(api.error))
    else:
        do_something_with(r)
    

    So why bother suppressing these errors in your API in the first place?

    Second, you replace all exceptions that you don't recognize with An unexpected error occurred. So this makes it impossible to distinguish a ProxyError from a TooManyRedirects from an SSLError.

    Third, you lose information that was present in the exception objects. For example, an SSLError comes with a description of the problem, for example if the certificate doesn't match the domain, you'll get an error like this:

    requests.exceptions.SSLError: hostname 'foo.com' doesn't match 'bar.com'
    

    This kind of information is vital in tracking down the cause of problems. But you replace this with An unexpected error occurred which is, frankly, useless.

  2. If the status code is not 200, you:

    raise Exception
    

    Which has three problems: (i) you should raise an instance of an exception class, not the class itself; (ii) you should initialize the exception object with data that describes the exception; and (iii) the class Exception is supposed to be the root of the class hierarchy for "built-in, non-system-exiting exceptions" and user exceptions. You shouldn't raise Exception itself, but instead derive a specific exception class and raise an instance of that. So here you would need something like:

    class HTTPError(Exception): pass
    

    And then:

    raise HTTPError('Status code {}'.format(r.status_code))
    

    But in fact the requests module already has a function for doing this, so all you need to do is:

    r.raise_for_status()
    
  3. Your interface for setting API parameters is to set them directly as properties of the API object:

    api = APIClient()
    api.userid = '123456'
    api.key = '0tKk8fAyHTlUv'
    api.base_url = 'https://api.com/'
    api.test_url = 'https://test.api.com/'
    api.testing = True
    

    It would be better for the constructor to take these as keyword arguments: (i) this allows the constructor to check that all required parameters have been set; (ii) parameter values can be checked (if possible); and (iii) the keyword argument mechanism is more flexible than object properties since you can pass a sets of keywords around in the form of a dictionary. So I would expect to write:

    api = APIClient(userid = '123456',
                    key = '0tKk8fAyHTlUv',
                    base_url = 'https://api.com/',
                    test_url = 'https://test.api.com/',
                    testing = True)
    
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the details, but about the exceptions, I'm suppressing the errors because I wanted to show these messages only to the users of the website, I don't want to print all the errors although I want to log all of them, so should I keep suppressing the exceptions or call api.request() in a try/except block ? \$\endgroup\$ – Pierre Nov 9 '13 at 11:27
  • 1
    \$\begingroup\$ Deciding how to handle exceptions should be up to the caller of the API, not up to the API itself. \$\endgroup\$ – Gareth Rees Nov 9 '13 at 11:46
  • \$\begingroup\$ Nice explanation, but i did't completely understood error handling part. Do you mean he should have only used one raise HTTPError('Status code {}'.format(r.status_code)) \$\endgroup\$ – Rizwan Mumtaz Jan 31 '17 at 19:15
  • \$\begingroup\$ And what is the good way to deal with validation errors \$\endgroup\$ – Rizwan Mumtaz Jan 31 '17 at 19:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.