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 resultAnd 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.errorMy 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.resultinstead 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?
- \$\begingroup\$Look at this package:github.com/olegpidsadnyi/restart\$\endgroup\$Markon– Markon2013-11-10 22:23:12 +00:00CommentedNov 10, 2013 at 22:23
1 Answer1
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
requestmethod and replace them with a string in the.errorproperty of theAPIClientobject.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 with
An unexpected error occurred.So this makes it impossible to distinguish aProxyErrorfrom aTooManyRedirectsfrom anSSLError.Third, you lose information that was present in the exception objects. For example, an
SSLErrorcomes 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 occurredwhich is, frankly, useless.If the status code is not 200, you:
raise ExceptionWhich 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 class
Exceptionis supposed to be theroot of the class hierarchy for "built-in, non-system-exiting exceptions" and user exceptions. You shouldn't raiseExceptionitself, but instead derive a specific exception class and raise an instance of that. So here you would need something like:class HTTPError(Exception): passAnd then:
raise HTTPError('Status code {}'.format(r.status_code))But in fact the
requestsmodulealready has a function for doing this, so all you need to do is:r.raise_for_status()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 = TrueIt 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)
- \$\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– Pierre2013-11-09 11:27:16 +00:00CommentedNov 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\$Gareth Rees– Gareth Rees2013-11-09 11:46:48 +00:00CommentedNov 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 one
raise HTTPError('Status code {}'.format(r.status_code))\$\endgroup\$Rizwan Mumtaz– Rizwan Mumtaz2017-01-31 19:15:01 +00:00CommentedJan 31, 2017 at 19:15 - \$\begingroup\$And what is the good way to deal with validation errors\$\endgroup\$Rizwan Mumtaz– Rizwan Mumtaz2017-01-31 19:16:09 +00:00CommentedJan 31, 2017 at 19:16
You mustlog in to answer this question.
Explore related questions
See similar questions with these tags.
