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 therequests library:

import requestsclass 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 clientapi = APIClient()# authenticationapi.userid = '123456'api.key = '0tKk8fAyHTlUv'# api urlsapi.base_url = 'https://api.com/'api.test_url = 'https://test.api.com/'# api in testing modeapi.testing = True# create a requestr = 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.errorelse:    print r# what about using something like ?:if api.request.ok:    print api.request.resultelse:    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 propertyapi.result instead of returning it when callingapi.request()?
  • How can I improve the way I get the API result? Is the second example better?
  • How can I improve my code?
Reinderien's user avatar
Reinderien
71.2k5 gold badges76 silver badges256 bronze badges
askedNov 6, 2013 at 11:32
Pierre's user avatar
\$\endgroup\$
1

1 Answer1

9
\$\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 yourrequest method and replace them with a string in the.error property of theAPIClient object.

    This has several problems.

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

    r = api.request(...) # might raise an exceptiondo_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 anyr = api.request(...) # might set api.errorif api.error:    # handle the error somehowelse:    do_something_with(r)

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

    api.error = None # clear the old error, if anyr = api.request(...) # might set api.errorif 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 withAn unexpected error occurred. So this makes it impossible to distinguish aProxyError from aTooManyRedirects from anSSLError.

    Third, you lose information that was present in the exception objects. For example, anSSLError 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 withAn 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 aninstance of an exception class, not the class itself; (ii) you should initialize the exception object with data that describes the exception; and (iii) the classException is supposed to be theroot of the class hierarchy for "built-in, non-system-exiting exceptions" and user exceptions. You shouldn't raiseException 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 therequests modulealready 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)
answeredNov 8, 2013 at 20:24
Gareth Rees's user avatar
\$\endgroup\$
4
  • \$\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 callapi.request() in a try/except block ?\$\endgroup\$CommentedNov 9, 2013 at 11:27
  • 1
    \$\begingroup\$Deciding how to handle exceptions should be up to thecaller of the API, not up to the API itself.\$\endgroup\$CommentedNov 9, 2013 at 11:46
  • \$\begingroup\$Nice explanation, but i did't completely understood error handling part. Do you mean he should have only used oneraise HTTPError('Status code {}'.format(r.status_code))\$\endgroup\$CommentedJan 31, 2017 at 19:15
  • \$\begingroup\$And what is the good way to deal with validation errors\$\endgroup\$CommentedJan 31, 2017 at 19:16

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.