diff --git a/HACKING.rst b/HACKING.rst index f2c06363e2..769d76efa3 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -21,4 +21,5 @@ glance Specific Commandments - [G323] Validate that LOG.exception messages use _LE. - [G324] Validate that LOG.error messages use _LE. - [G325] Validate that LOG.critical messages use _LC. -- [G326] Validate that LOG.warning messages use _LW. \ No newline at end of file +- [G326] Validate that LOG.warning messages use _LW. +- [G327] Prevent use of deprecated contextlib.nested \ No newline at end of file diff --git a/glance/hacking/checks.py b/glance/hacking/checks.py index a79f3f59ae..bb41e7c251 100644 --- a/glance/hacking/checks.py +++ b/glance/hacking/checks.py @@ -139,6 +139,15 @@ def validate_log_translations(logical_line, physical_line, filename): yield (0, msg) +def check_no_contextlib_nested(logical_line): + msg = ("G327: contextlib.nested is deprecated since Python 2.7. See " + "https://docs.python.org/2/library/contextlib.html#contextlib." + "nested for more information.") + if ("with contextlib.nested(" in logical_line or + "with nested(" in logical_line): + yield(0, msg) + + def factory(register): register(assert_true_instance) register(assert_equal_type) @@ -146,3 +155,4 @@ def factory(register): register(no_translate_debug_logs) register(no_direct_use_of_unicode_function) register(validate_log_translations) + register(check_no_contextlib_nested) diff --git a/glance/tests/test_hacking.py b/glance/tests/test_hacking.py index 9ad47d8000..685185d710 100644 --- a/glance/tests/test_hacking.py +++ b/glance/tests/test_hacking.py @@ -62,3 +62,13 @@ class HackingTestCase(utils.BaseTestCase): "six.text_type('party over')")))) self.assertEqual(0, len(list(checks.no_direct_use_of_unicode_function( "not_actually_unicode('something completely different')")))) + + def test_no_contextlib_nested(self): + self.assertEqual(1, len(list(checks.check_no_contextlib_nested( + "with contextlib.nested(")))) + + self.assertEqual(1, len(list(checks.check_no_contextlib_nested( + "with nested(")))) + + self.assertEqual(0, len(list(checks.check_no_contextlib_nested( + "with foo as bar")))) diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py index f64411b944..3996762e9e 100644 --- a/glance/tests/unit/v1/test_api.py +++ b/glance/tests/unit/v1/test_api.py @@ -15,7 +15,6 @@ # License for the specific language governing permissions and limitations # under the License. -import contextlib import copy import datetime import hashlib @@ -394,7 +393,10 @@ class TestGlanceAPI(base.IsolatedUnitTest): self.assertEqual(400, res.status_int) self.assertIn('Required store bad is invalid', res.body) - def test_create_with_location_get_store_or_400_raises_exception(self): + @mock.patch.object(glance.api.v1.images.Controller, '_external_source') + @mock.patch.object(store, 'get_store_from_location') + def test_create_with_location_get_store_or_400_raises_exception( + self, mock_get_store_from_location, mock_external_source): location = 'bad+scheme://localhost:0/image.qcow2' scheme = 'bad+scheme' fixture_headers = { @@ -409,23 +411,14 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in six.iteritems(fixture_headers): req.headers[k] = v - ctlr = glance.api.v1.images.Controller + mock_external_source.return_value = location + mock_get_store_from_location.return_value = scheme - with contextlib.nested( - mock.patch.object(ctlr, '_external_source', - return_value=location), - mock.patch.object(store, - 'get_store_from_location', - return_value=scheme) - ) as ( - mock_external_source, - mock_get_store_from_location - ): - res = req.get_response(self.api) - self.assertEqual(400, res.status_int) - self.assertEqual(1, mock_external_source.call_count) - self.assertEqual(1, mock_get_store_from_location.call_count) - self.assertIn('Store for scheme %s not found' % scheme, res.body) + res = req.get_response(self.api) + self.assertEqual(400, res.status_int) + self.assertEqual(1, mock_external_source.call_count) + self.assertEqual(1, mock_get_store_from_location.call_count) + self.assertIn('Store for scheme %s not found' % scheme, res.body) def test_create_with_location_unknown_scheme(self): fixture_headers = {