x/vmware-nsx
Code Issues Proposed changes

Do not mask NotImplementedError exceptions

Browse Source 
With this patch we  avoid masking NotImplementedError(s).
Previously, a catch-all clause returned 500, and it makes
sense to distinguish between genuine 500 errors (i.e. a bug),
versus 501 ones.

This opens up the issue of keeping the server behavior
consistent from one release to another, but one might argue
that this was bad design decision in the first place.

Fixes bug 1197200

Change-Id: I2d81cba5ee9e5fbe8e865378381790b5b467d255
This commit is contained in:
armando-migliaccio 2013-07-02 17:52:39 -07:00
parent 0fc66059af
commit 6ab8dd80f6
2 changed files with 14 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
quantum/api/v2/resource.py
View File

@ -94,6 +94,17 @@ def Resource(controller, faults=None, deserializers=None, serializers=None):
e.body = serializer.serialize({'QuantumError': e})
e.content_type = content_type
raise
except NotImplementedError as e:
# NOTE(armando-migliaccio): from a client standpoint
# it makes sense to receive these errors, because
# extensions may or may not be implemented by
# the underlying plugin. So if something goes south,
# because a plugin does not implement a feature,
# returning 500 is definitely confusing.
body = serializer.serialize(
{'NotImplementedError': e.message})
kwargs = {'body': body, 'content_type': content_type}
raise webob.exc.HTTPNotImplemented(**kwargs)
except Exception as e:
# NOTE(jkoelker) Everyting else is 500
LOG.exception(_('%s failed'), action)

6
quantum/tests/unit/test_extensions.py
View File

@ -76,7 +76,7 @@ class ResourceExtensionTest(base.BaseTestCase):
return {'data': {'id': id}}
def notimplemented_function(self, request, id):
return webob.exc.HTTPClientError(NotImplementedError())
return webob.exc.HTTPNotImplemented()
def custom_member_action(self, request, id):
return {'member_action': 'value'}
@ -114,8 +114,8 @@ class ResourceExtensionTest(base.BaseTestCase):
test_app.get("/tweedles/some_id/notimplemented_function")
# Shouldn't be reached
self.assertTrue(False)
except webtest.AppError:
pass
except webtest.AppError as e:
self.assertTrue('501' in e.message)
def test_resource_can_be_added_as_extension(self):
res_ext = extensions.ResourceExtension(