From a8edbd2e8d4cd6254531bcfaf7ae480338249738 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 7 Jul 2016 13:14:40 +0200
Subject: [PATCH] Improve api_version decorator to avoid noqa
Our existing `api_version` decorator for microversions forces us to use
"# noqa" if we have multiple microversions of a method in the same
class, providing no way around this.
This patch adds a way to avoid using noqa directive. Previously we
would have:
@wsgi.Controller.api_version("3.3")
def my_api_method(self, req, id):
.... method_1 ...
@wsgi.Controller.api_version("3.4") # noqa
def my_api_method(self, req, id):
.... method_2 ...
With this patch the second method does not require noqa anymore:
@my_api_method.api_version("3.4")
def my_api_method(self, req, id):
.... method_2 ...
Devref is updated to reflect this new change and to mention that it is
the recommended way to do it.
Change-Id: I46283b52cc6a347d5deb0f57a123eba4e01a3eb2
Closes-Bug: #1599806
---
cinder/api/openstack/wsgi.py | 6 ++++++
cinder/api/versions.py | 4 ++--
cinder/tests/unit/api/test_versions.py | 11 +++++++----
doc/source/devref/api_microversion_dev.rst | 15 ++++++++++-----
4 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py
index cc4718e55fd..76d92f21ece 100644
--- a/cinder/api/openstack/wsgi.py
+++ b/cinder/api/openstack/wsgi.py
@@ -1220,6 +1220,12 @@ class Controller(object):
# ranges of valid versions as that is ambiguous
func_list.sort(reverse=True)
+ # NOTE(geguileo): To avoid PEP8 errors when defining multiple
+ # microversions of the same method in the same class we add the
+ # api_version decorator to the function so it can be used instead,
+ # thus preventing method redefinition errors.
+ f.api_version = cls.api_version
+
return f
return decorator
diff --git a/cinder/api/versions.py b/cinder/api/versions.py
index 60413d3b43a..d6acf214ce9 100644
--- a/cinder/api/versions.py
+++ b/cinder/api/versions.py
@@ -100,7 +100,7 @@ class VersionsController(wsgi.Controller):
known_versions.pop('v3.0')
return builder.build_versions(known_versions)
- @wsgi.Controller.api_version('2.0') # noqa
+ @index.api_version('2.0')
def index(self, req): # pylint: disable=E0102
"""Return versions supported prior to the microversions epoch."""
builder = views_versions.get_view_builder(req)
@@ -109,7 +109,7 @@ class VersionsController(wsgi.Controller):
known_versions.pop('v3.0')
return builder.build_versions(known_versions)
- @wsgi.Controller.api_version('3.0') # noqa
+ @index.api_version('3.0')
def index(self, req): # pylint: disable=E0102
"""Return versions supported after the start of microversions."""
builder = views_versions.get_view_builder(req)
diff --git a/cinder/tests/unit/api/test_versions.py b/cinder/tests/unit/api/test_versions.py
index 1d3dc9c842a..d254de1db50 100644
--- a/cinder/tests/unit/api/test_versions.py
+++ b/cinder/tests/unit/api/test_versions.py
@@ -262,7 +262,8 @@ class VersionsControllerTestCase(test.TestCase):
('3.2', 'index', 'child 3.2', 'ControllerChild'),
('3.2', 'show', 404, 'ControllerChild'),
('3.3', 'index', 'child 3.3', 'ControllerChild'),
- ('3.3', 'show', 'show', 'ControllerChild'))
+ ('3.3', 'show', 'show', 'ControllerChild'),
+ ('3.4', 'index', 'child 3.4', 'ControllerChild'))
@ddt.unpack
def test_versions_inheritance_of_non_base_controller(self, version, call,
expected, controller):
@@ -287,12 +288,14 @@ class VersionsControllerTestCase(test.TestCase):
def index(self, req):
return 'child 3.2'
- # TODO(geguileo): Figure out a way to make microversions work in a
- # way that doesn't raise complaints from duplicated method.
- @wsgi.Controller.api_version('3.3') # noqa
+ @index.api_version('3.3')
def index(self, req):
return 'child 3.3'
+ @index.api_version('3.4')
+ def index(self, req):
+ return 'child 3.4'
+
@wsgi.Controller.api_version('3.3')
def show(self, req, *args, **kwargs):
return 'show'
diff --git a/doc/source/devref/api_microversion_dev.rst b/doc/source/devref/api_microversion_dev.rst
index 9f895682909..5adab6ccb6b 100644
--- a/doc/source/devref/api_microversion_dev.rst
+++ b/doc/source/devref/api_microversion_dev.rst
@@ -186,7 +186,7 @@ In the controller class::
def my_api_method(self, req, id):
.... method_1 ...
- @wsgi.Controller.api_version("3.4") # noqa
+ @my_api_method.api_version("3.4")
def my_api_method(self, req, id):
.... method_2 ...
@@ -194,10 +194,15 @@ If a caller specified ``3.1``, ``3.2`` or ``3.3`` (or received the
default of ``3.1``) they would see the result from ``method_1``,
``3.4`` or later ``method_2``.
-It is vital that the two methods have the same name, so the second of
-them will need ``# noqa`` to avoid failing flake8's ``F811`` rule. The
-two methods may be different in any kind of semantics (schema
-validation, return values, response codes, etc)
+We could use ``wsgi.Controller.api_version`` decorator on the second
+``my_api_method`` as well, but then we would have to add ``# no qa`` to that
+line to avoid failing flake8's ``F811`` rule. So the recommended approach is
+to use the ``api_version`` decorator from the first method that is defined, as
+illustrated by the example above, and then use ``my_api_method`` decorator for
+subsequent api versions of the same method.
+
+The two methods may be different in any kind of semantics (schema validation,
+return values, response codes, etc.).
A method with only small changes between versions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~