openstack/python-swiftclient
Code Issues Proposed changes

make ClientException.http_status default to None rather than 0

Browse Source 
The extant default of zero is a bit counterintuitive;
insufficiently-careful programmers using swiftclient in their
application might, without carefully reading the source or
documentation, write buggy code based on the assumption that the
`http_status` attribute is absent or defaults to None if ClientException
is raised for reasons other than to indicate an unsuccessful HTTP
request. (However improbable this scenario may seem, the present author
can sadly attest to it having actually happened at least once.)

Just changing the default would break some tests on Python 3, due to the
`500 <= err.http_status <= 599` comparison in Connection's _retry
method (NoneType and int are not orderable in the Python 3.x series);
thus, the case where http_status is None is explicitly folded into a
code branch that logs and reraises (whereas previously it would have
fallen through to an `else` branch where it would be logged and reraised
just the same).

While we're here, we might as well make ClientException's __init__ use
super() (although admittedly the kinds of multiple-inheritance scenarios
in which `super` truly shines seem unlikely to occur here).

Change-Id: I8c02bfb4a0ef059e781be5e08fcde13fb1be5b88
This commit is contained in:
Zack M. Davis 2015-09-04 15:45:48 -07:00
parent 93666bb84a
commit 5ae4b42392
2 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
swiftclient/client.py
View File

@ -1400,7 +1400,7 @@ class Connection(object):
self.http_conn = None
except ClientException as err:
self._add_response_dict(caller_response_dict, kwargs)
if self.attempts > self.retries:
if self.attempts > self.retries or err.http_status is None:
logger.exception(err)
raise
if err.http_status == 401:

4
swiftclient/exceptions.py
View File

@ -17,9 +17,9 @@
class ClientException(Exception):
def __init__(self, msg, http_scheme='', http_host='', http_port='',
http_path='', http_query='', http_status=0, http_reason='',
http_path='', http_query='', http_status=None, http_reason='',
http_device='', http_response_content=''):
Exception.__init__(self, msg)
super(ClientException, self).__init__(msg)
self.msg = msg
self.http_scheme = http_scheme
self.http_host = http_host