From 198a3bea231883251ef00596ddda625f33219efd Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Mon, 9 Mar 2015 02:19:32 +0000 Subject: [PATCH] Add a hacking rule for consistent HTTP501 message There is raise_feature_not_supported() for returning a HTTP501 response with consistent error message, and this patch adds a rule for enforcing to use the method on v2.1 API. Partially implements blueprint v2-on-v3-api Change-Id: I06f254fd9c8d8b6aac4ed135c6c407f3a993431f --- HACKING.rst | 1 + nova/hacking/checks.py | 13 +++++++++++++ nova/tests/unit/test_hacking.py | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/HACKING.rst b/HACKING.rst index 97cf0f9d0305..42a0a4581dc5 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -50,6 +50,7 @@ Nova Specific Commandments - [N338] Change assertEqual(A in B, True), assertEqual(True, A in B), assertEqual(A in B, False) or assertEqual(False, A in B) to the more specific assertIn/NotIn(A, B) +- [N339] Check common raise_feature_not_supported() is used for v2.1 HTTPNotImplemented response. Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 3f9440cc2f67..8d5b6135c482 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -95,6 +95,7 @@ custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") api_version_re = re.compile(r"@.*api_version") dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") decorator_re = re.compile(r"@.*") +http_not_implemented_re = re.compile(r"raise .*HTTPNotImplemented\(") # TODO(dims): When other oslo libraries switch over non-namespace'd # imports, we need to add them to the regexp below. @@ -539,6 +540,17 @@ def assert_equal_in(logical_line): "contents.") +def check_http_not_implemented(logical_line, physical_line, filename): + msg = ("N339: HTTPNotImplemented response must be implemented with" + " common raise_feature_not_supported().") + if pep8.noqa(physical_line): + return + if "nova/api/openstack/compute/plugins/v3" not in filename: + return + if re.match(http_not_implemented_re, logical_line): + yield(0, msg) + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -565,3 +577,4 @@ def factory(register): register(assert_true_or_false_with_in) register(dict_constructor_with_list_copy) register(assert_equal_in) + register(check_http_not_implemented) diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index c469f12de88c..7dbdf827ce71 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -533,3 +533,25 @@ class HackingTestCase(test.NoDBTestCase): self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( " self._render_dict(xml, data_el, data.__dict__)")))) + + def test_check_http_not_implemented(self): + code = """ + except NotImplementedError: + common.raise_http_not_implemented_error() + """ + filename = "nova/api/openstack/compute/plugins/v3/test.py" + self._assert_has_no_errors(code, checks.check_http_not_implemented, + filename=filename) + + code = """ + except NotImplementedError: + msg = _("Unable to set password on instance") + raise exc.HTTPNotImplemented(explanation=msg) + """ + errors = [(3, 4, 'N339')] + self._assert_has_errors(code, checks.check_http_not_implemented, + expected_errors=errors, filename=filename) + + filename = "nova/api/openstack/compute/contrib/test.py" + self._assert_has_no_errors(code, checks.check_http_not_implemented, + filename=filename)