Merge "Quality of life improvements to APIVersionManager"

horizon/exceptions.py

@@ -172,6 +172,11 @@ class AlreadyExists(HorizonException):
         return self.msg % self.attrs
 
 
+class ConfigurationError(HorizonException):
+    """Exception to be raised when invalid settings have been provided."""
+    pass
+
+
 class NotAvailable(HorizonException):
     """Exception to be raised when something is not available."""
     pass

openstack_dashboard/api/base.py

@@ -23,6 +23,8 @@ from django.conf import settings
 
 from horizon import exceptions
 
+import six
+
 
 __all__ = ('APIResourceWrapper', 'APIDictWrapper',
            'get_service_from_catalog', 'url_for',)
@@ -41,6 +43,13 @@ class APIVersionManager(object):
         self.preferred = preferred_version
         self._active = None
         self.supported = {}
+        # As a convenience, we can drop in a placeholder for APIs that we
+        # have not yet needed to version. This is useful, for example, when
+        # panels such as the admin metadata_defs wants to check the active
+        # version even though it's not explicitly defined. Previously
+        # this caused a KeyError.
+        if self.preferred:
+            self.supported[self.preferred] = {"version": self.preferred}
 
     @property
     def active(self):
@@ -60,9 +69,26 @@ class APIVersionManager(object):
             # the setting in as a way of overriding the latest available
             # version.
             key = self.preferred
+        # Since we do a key lookup in the supported dict the type matters,
+        # let's ensure people know if they use a string when the key isn't.
+        if isinstance(key, six.string_types):
+            msg = ('The version "%s" specified for the %s service should be '
+                   'either an integer or a float, not a string.' %
+                   (key, self.service_type))
+            raise exceptions.ConfigurationError(msg)
+        # Provide a helpful error message if the specified version isn't in the
+        # supported list.
+        if key not in self.supported:
+            choices = ", ".join(str(k) for k in six.iterkeys(self.supported))
+            msg = ('%s is not a supported API version for the %s service, '
+                   ' choices are: %s' % (key, self.service_type, choices))
+            raise exceptions.ConfigurationError(msg)
         self._active = key
         return self.supported[self._active]
 
+    def clear_active_cache(self):
+        self._active = None
+
 
 class APIResourceWrapper(object):
     """Simple wrapper for api objects.

openstack_dashboard/local/local_settings.py.example

@@ -28,9 +28,10 @@ TEMPLATE_DEBUG = DEBUG
 
 # Overrides for OpenStack API versions. Use this setting to force the
 # OpenStack dashboard to use a specific API version for a given service API.
+# Versions specified here should be integers or floats, not strings.
 # NOTE: The version should be formatted as it appears in the URL for the
 # service API. For example, The identity service APIs have inconsistent
-# use of the decimal point, so valid options would be "2.0" or "3".
+# use of the decimal point, so valid options would be 2.0 or 3.
 # OPENSTACK_API_VERSIONS = {
 #     "data_processing": 1.1,
 #     "identity": 3,

openstack_dashboard/test/api_tests/base_tests.py

@@ -18,9 +18,14 @@
 
 from __future__ import absolute_import
 
+from django.conf import settings
+
 from horizon import exceptions
 
 from openstack_dashboard.api import base as api_base
+from openstack_dashboard.api import cinder
+from openstack_dashboard.api import glance
+from openstack_dashboard.api import keystone
 from openstack_dashboard.test import helpers as test
 
 
@@ -147,6 +152,39 @@ class APIDictWrapperTests(test.TestCase):
         self.assertFalse(0 in resource)
 
 
+class ApiVersionTests(test.TestCase):
+    def setUp(self):
+        super(ApiVersionTests, self).setUp()
+        self.previous_settings = settings.OPENSTACK_API_VERSIONS
+        settings.OPENSTACK_API_VERSIONS = {
+            "data_processing": 1.1,
+            "identity": "2.0",
+            "volume": 1
+        }
+        # Make sure cached data from other tests doesn't interfere
+        cinder.VERSIONS.clear_active_cache()
+        keystone.VERSIONS.clear_active_cache()
+        glance.VERSIONS.clear_active_cache()
+
+    def tearDown(self):
+        super(ApiVersionTests, self).tearDown()
+        settings.OPENSTACK_API_VERSIONS = self.previous_settings
+        # Clear out our bogus data so it doesn't interfere
+        cinder.VERSIONS.clear_active_cache()
+        keystone.VERSIONS.clear_active_cache()
+        glance.VERSIONS.clear_active_cache()
+
+    def test_invalid_versions(self):
+        with self.assertRaises(exceptions.ConfigurationError):
+            getattr(keystone.VERSIONS, 'active')
+        with self.assertRaises(exceptions.ConfigurationError):
+            getattr(cinder.VERSIONS, 'active')
+        try:
+            getattr(glance.VERSIONS, 'active')
+        except exceptions.ConfigurationError:
+            self.fail("ConfigurationError raised inappropriately.")
+
+
 class ApiHelperTests(test.TestCase):
     """Tests for functions that don't use one of the api objects."""