x/swift3
Code Issues Proposed changes

Use InternalError for unexpected server errors

Browse Source 
'InvalidURI' doesn't give correct information about what happened.
'InternalError' is much better.

Change-Id: Id88ac87d6097dc8d2ff67bcad28059f8ea187ec4
This commit is contained in:
MORITA Kazutaka 2014-06-05 10:19:29 +09:00
parent e3ee641242
commit 2e78cd0355
2 changed files with 28 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
swift3/middleware.py
View File

@ -65,7 +65,8 @@ import re
from swift.common.utils import get_logger from swift.common.utils import get_logger
from swift.common.swob import Request, Response, HTTPForbidden, HTTPConflict, \ from swift.common.swob import Request, Response, HTTPForbidden, HTTPConflict, \
HTTPBadRequest, HTTPMethodNotAllowed, HTTPNotFound, HTTPNotImplemented, \ HTTPBadRequest, HTTPMethodNotAllowed, HTTPNotFound, HTTPNotImplemented, \
HTTPLengthRequired, HTTPServiceUnavailable, HTTPNoContent, HTTPOk HTTPLengthRequired, HTTPServiceUnavailable, HTTPNoContent, HTTPOk, \
HTTPInternalServerError
from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \ from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \ HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \
HTTP_CONFLICT, HTTP_UNPROCESSABLE_ENTITY, is_success, \ HTTP_CONFLICT, HTTP_UNPROCESSABLE_ENTITY, is_success, \
@ -97,6 +98,9 @@ def get_err_response(code):
(HTTPConflict, 'The requested bucket name is not available'), (HTTPConflict, 'The requested bucket name is not available'),
'BucketNotEmpty': 'BucketNotEmpty':
(HTTPConflict, 'The bucket you tried to delete is not empty'), (HTTPConflict, 'The bucket you tried to delete is not empty'),
'InternalError':
(HTTPInternalServerError, 'We encountered an internal error. '
'Please try again.'),
'InvalidArgument': 'InvalidArgument':
(HTTPBadRequest, 'Invalid Argument'), (HTTPBadRequest, 'Invalid Argument'),
'InvalidBucketName': 'InvalidBucketName':
@ -406,7 +410,7 @@ class ServiceController(Controller):
if status in (HTTP_UNAUTHORIZED, HTTP_FORBIDDEN): if status in (HTTP_UNAUTHORIZED, HTTP_FORBIDDEN):
return get_err_response('AccessDenied') return get_err_response('AccessDenied')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
containers = loads(resp.body) containers = loads(resp.body)
# we don't keep the creation time of a backet (s3cmd doesn't # we don't keep the creation time of a backet (s3cmd doesn't
@ -474,7 +478,7 @@ class BucketController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchBucket') return get_err_response('NoSuchBucket')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
objects = loads(resp.body) objects = loads(resp.body)
body = ('<?xml version="1.0" encoding="UTF-8"?>' body = ('<?xml version="1.0" encoding="UTF-8"?>'
@ -548,7 +552,7 @@ class BucketController(Controller):
elif status == HTTP_ACCEPTED: elif status == HTTP_ACCEPTED:
return get_err_response('BucketAlreadyExists') return get_err_response('BucketAlreadyExists')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
return HTTPOk(headers={'Location': self.container_name}) return HTTPOk(headers={'Location': self.container_name})
@ -567,7 +571,7 @@ class BucketController(Controller):
elif status == HTTP_CONFLICT: elif status == HTTP_CONFLICT:
return get_err_response('BucketNotEmpty') return get_err_response('BucketNotEmpty')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
return HTTPNoContent() return HTTPNoContent()
@ -607,7 +611,7 @@ class ObjectController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchKey') return get_err_response('NoSuchKey')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
def HEAD(self, req): def HEAD(self, req):
""" """
@ -655,7 +659,7 @@ class ObjectController(Controller):
elif status == HTTP_REQUEST_ENTITY_TOO_LARGE: elif status == HTTP_REQUEST_ENTITY_TOO_LARGE:
return get_err_response('EntityTooLarge') return get_err_response('EntityTooLarge')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
if 'HTTP_X_COPY_FROM' in req.environ: if 'HTTP_X_COPY_FROM' in req.environ:
body = '<CopyObjectResult>' \ body = '<CopyObjectResult>' \
@ -675,7 +679,7 @@ class ObjectController(Controller):
try: try:
resp = req.get_response(self.app) resp = req.get_response(self.app)
except Exception: except Exception:
return get_err_response('InvalidURI') return get_err_response('InternalError')
status = resp.status_int status = resp.status_int
@ -685,7 +689,7 @@ class ObjectController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchKey') return get_err_response('NoSuchKey')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
return HTTPNoContent() return HTTPNoContent()
@ -726,7 +730,7 @@ class AclController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchKey') return get_err_response('NoSuchKey')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
else: else:
# Handle Bucket ACL # Handle Bucket ACL
@ -742,7 +746,7 @@ class AclController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchBucket') return get_err_response('NoSuchBucket')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
def PUT(self, req): def PUT(self, req):
""" """
@ -773,7 +777,7 @@ class AclController(Controller):
elif status == HTTP_ACCEPTED: elif status == HTTP_ACCEPTED:
return get_err_response('BucketAlreadyExists') return get_err_response('BucketAlreadyExists')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
return HTTPOk(headers={'Location': self.container_name}) return HTTPOk(headers={'Location': self.container_name})
@ -796,7 +800,7 @@ class LocationController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchBucket') return get_err_response('NoSuchBucket')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
body = ('<?xml version="1.0" encoding="UTF-8"?>' body = ('<?xml version="1.0" encoding="UTF-8"?>'
'<LocationConstraint ' '<LocationConstraint '
@ -830,7 +834,7 @@ class LoggingStatusController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchBucket') return get_err_response('NoSuchBucket')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
# logging disabled # logging disabled
body = ('<?xml version="1.0" encoding="UTF-8"?>' body = ('<?xml version="1.0" encoding="UTF-8"?>'
@ -901,7 +905,8 @@ class MultiObjectDeleteController(Controller):
if status == HTTP_UNAUTHORIZED: if status == HTTP_UNAUTHORIZED:
body += get_err_elem(key, 'AccessDenied', 'Access Denied') body += get_err_elem(key, 'AccessDenied', 'Access Denied')
else: else:
body += get_err_elem(key, 'InvalidURI', 'Invalid URI') body += get_err_elem(key, 'InternalError',
'Internal Error')
body += '</DeleteResult>\r\n' body += '</DeleteResult>\r\n'
return HTTPOk(body=body) return HTTPOk(body=body)
@ -1002,7 +1007,7 @@ class VersioningController(Controller):
elif status == HTTP_NOT_FOUND: elif status == HTTP_NOT_FOUND:
return get_err_response('NoSuchBucket') return get_err_response('NoSuchBucket')
else: else:
return get_err_response('InvalidURI') return get_err_response('InternalError')
# Just report there is no versioning configured here. # Just report there is no versioning configured here.
body = ('<VersioningConfiguration ' body = ('<VersioningConfiguration '

14
swift3/test/unit/test_swift3.py
View File

@ -209,7 +209,7 @@ class TestSwift3(unittest.TestCase):
code = self._test_method_error('GET', '', swob.HTTPForbidden) code = self._test_method_error('GET', '', swob.HTTPForbidden)
self.assertEquals(code, 'AccessDenied') self.assertEquals(code, 'AccessDenied')
code = self._test_method_error('GET', '', swob.HTTPServerError) code = self._test_method_error('GET', '', swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_service_GET(self): def test_service_GET(self):
req = Request.blank('/', req = Request.blank('/',
@ -242,7 +242,7 @@ class TestSwift3(unittest.TestCase):
code = self._test_method_error('GET', '/bucket', swob.HTTPNotFound) code = self._test_method_error('GET', '/bucket', swob.HTTPNotFound)
self.assertEquals(code, 'NoSuchBucket') self.assertEquals(code, 'NoSuchBucket')
code = self._test_method_error('GET', '/bucket', swob.HTTPServerError) code = self._test_method_error('GET', '/bucket', swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_bucket_GET(self): def test_bucket_GET(self):
bucket_name = 'junk' bucket_name = 'junk'
@ -356,7 +356,7 @@ class TestSwift3(unittest.TestCase):
code = self._test_method_error('PUT', '/bucket', swob.HTTPAccepted) code = self._test_method_error('PUT', '/bucket', swob.HTTPAccepted)
self.assertEquals(code, 'BucketAlreadyExists') self.assertEquals(code, 'BucketAlreadyExists')
code = self._test_method_error('PUT', '/bucket', swob.HTTPServerError) code = self._test_method_error('PUT', '/bucket', swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_bucket_PUT(self): def test_bucket_PUT(self):
req = Request.blank('/bucket', req = Request.blank('/bucket',
@ -377,7 +377,7 @@ class TestSwift3(unittest.TestCase):
self.assertEquals(code, 'BucketNotEmpty') self.assertEquals(code, 'BucketNotEmpty')
code = self._test_method_error('DELETE', '/bucket', code = self._test_method_error('DELETE', '/bucket',
swob.HTTPServerError) swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_bucket_DELETE(self): def test_bucket_DELETE(self):
req = Request.blank('/bucket', req = Request.blank('/bucket',
@ -447,7 +447,7 @@ class TestSwift3(unittest.TestCase):
self.assertEquals(code, 'NoSuchKey') self.assertEquals(code, 'NoSuchKey')
code = self._test_method_error('GET', '/bucket/object', code = self._test_method_error('GET', '/bucket/object',
swob.HTTPServerError) swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_object_GET(self): def test_object_GET(self):
self._test_object_GETorHEAD('GET') self._test_object_GETorHEAD('GET')
@ -478,7 +478,7 @@ class TestSwift3(unittest.TestCase):
self.assertEquals(code, 'EntityTooLarge') self.assertEquals(code, 'EntityTooLarge')
code = self._test_method_error('PUT', '/bucket/object', code = self._test_method_error('PUT', '/bucket/object',
swob.HTTPServerError) swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_object_PUT(self): def test_object_PUT(self):
req = Request.blank( req = Request.blank(
@ -525,7 +525,7 @@ class TestSwift3(unittest.TestCase):
self.assertEquals(code, 'NoSuchKey') self.assertEquals(code, 'NoSuchKey')
code = self._test_method_error('DELETE', '/bucket/object', code = self._test_method_error('DELETE', '/bucket/object',
swob.HTTPServerError) swob.HTTPServerError)
self.assertEquals(code, 'InvalidURI') self.assertEquals(code, 'InternalError')
def test_object_DELETE(self): def test_object_DELETE(self):
req = Request.blank('/bucket/object', req = Request.blank('/bucket/object',