Deprecate ServiceCatalog(region_name)

There was a FIXME to deprecate ServiceCatalog's region_name
parameter and property. This is now deprecated.

Note that debtcollector isn't used here since the deprecation
happens on ServiceCatalog's __init__() to catch use in
subclasses of ServiceCatalog. ServiceCatalog also has a
factory function that constructs the correct instance and
the factory function always passes region_name, so it's
always using the deprecated kwarg even when region_name
isn't passed to the factory. It's not worth figuring out
how to do this with debtcollector.

bp deprecations

Change-Id: I0e64712474ca2767f3c0ade919359132450f6776

keystoneclient/httpclient.py

@@ -279,8 +279,13 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
             self._management_url = management_urls[0]
             self.auth_token_from_user = self.auth_ref.auth_token
             self.trust_id = self.auth_ref.trust_id
+
+            # TODO(blk-u): Using self.auth_ref.service_catalog._region_name is
+            # deprecated and this code must be removed when the property is
+            # actually removed.
             if self.auth_ref.has_service_catalog() and not region_name:
-                region_name = self.auth_ref.service_catalog.region_name
+                region_name = self.auth_ref.service_catalog._region_name
+
         else:
             self.auth_ref = None
 

keystoneclient/service_catalog.py

@@ -17,6 +17,7 @@
 # limitations under the License.
 
 import abc
+import warnings
 
 import six
 
@@ -27,11 +28,27 @@ from keystoneclient import utils
 
 @six.add_metaclass(abc.ABCMeta)
 class ServiceCatalog(object):
-    """Helper methods for dealing with a Keystone Service Catalog."""
+    """Helper methods for dealing with a Keystone Service Catalog.
+
+    .. warning::
+
+        Setting region_name is deprecated in favor of passing the region name
+        as a parameter to calls made to the service catalog as of the 1.7.0
+        release and may be removed in the 2.0.0 release.
+
+    """
 
     @classmethod
     def factory(cls, resource_dict, token=None, region_name=None):
-        """Create ServiceCatalog object given an auth token."""
+        """Create ServiceCatalog object given an auth token.
+
+        .. warning::
+
+            Setting region_name is deprecated in favor of passing the region
+            name as a parameter to calls made to the service catalog as of the
+            1.7.0 release and may be removed in the 2.0.0 release.
+
+        """
         if ServiceCatalogV3.is_valid(resource_dict):
             return ServiceCatalogV3(token, resource_dict, region_name)
         elif ServiceCatalogV2.is_valid(resource_dict):
@@ -40,13 +57,32 @@ class ServiceCatalog(object):
             raise NotImplementedError(_('Unrecognized auth response'))
 
     def __init__(self, region_name=None):
+        if region_name:
+            warnings.warn(
+                'Setting region_name on the service catalog is deprecated in '
+                'favor of passing the region name as a parameter to calls '
+                'made to the service catalog as of the 1.7.0 release and may '
+                'be removed in the 2.0.0 release.',
+                DeprecationWarning)
+
         self._region_name = region_name
 
     @property
     def region_name(self):
-        # FIXME(jamielennox): Having region_name set on the service catalog
-        # directly is deprecated. It should instead be provided as a parameter
-        # to calls made to the service_catalog. Provide appropriate warning.
+        """Region name.
+
+        .. warning::
+
+            region_name is deprecated in favor of passing the region name as a
+            parameter to calls made to the service catalog as of the 1.7.0
+            release and may be removed in the 2.0.0 release.
+
+        """
+        warnings.warn(
+            'region_name is deprecated in favor of passing the region name as '
+            'a parameter to calls made to the service catalog as of the 1.7.0 '
+            'release and may be removed in the 2.0.0 release.',
+            DeprecationWarning)
         return self._region_name
 
     def _get_endpoint_region(self, endpoint):

keystoneclient/tests/unit/v2_0/test_service_catalog.py

@@ -48,7 +48,9 @@ class ServiceCatalogTest(utils.TestCase):
 
     def test_service_catalog_regions(self):
         self.AUTH_RESPONSE_BODY['access']['region_name'] = "North"
-        auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
+        # Setting region_name on the catalog is deprecated.
+        with self.deprecations.expect_deprecations_here():
+            auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
         sc = auth_ref.service_catalog
 
         url = sc.url_for(service_type='image', endpoint_type='publicURL')
@@ -133,7 +135,9 @@ class ServiceCatalogTest(utils.TestCase):
 
     def test_service_catalog_param_overrides_body_region(self):
         self.AUTH_RESPONSE_BODY['access']['region_name'] = "North"
-        auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
+        # Setting region_name on the catalog is deprecated.
+        with self.deprecations.expect_deprecations_here():
+            auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
         sc = auth_ref.service_catalog
 
         url = sc.url_for(service_type='image')

keystoneclient/tests/unit/v3/test_client.py

@@ -191,6 +191,7 @@ class KeystoneClientTest(utils.TestCase):
     def test_client_with_region_name_passes_to_service_catalog(self):
         # NOTE(jamielennox): this is deprecated behaviour that should be
         # removed ASAP, however must remain compatible.
+        self.deprecations.expect_deprecations()
 
         self.stub_auth(json=client_fixtures.auth_response_body())
 

keystoneclient/tests/unit/v3/test_service_catalog.py

@@ -66,8 +66,10 @@ class ServiceCatalogTest(utils.TestCase):
 
     def test_service_catalog_regions(self):
         self.AUTH_RESPONSE_BODY['token']['region_name'] = "North"
-        auth_ref = access.AccessInfo.factory(self.RESPONSE,
-                                             self.AUTH_RESPONSE_BODY)
+        # Setting region_name on the catalog is deprecated.
+        with self.deprecations.expect_deprecations_here():
+            auth_ref = access.AccessInfo.factory(self.RESPONSE,
+                                                 self.AUTH_RESPONSE_BODY)
         sc = auth_ref.service_catalog
 
         url = sc.url_for(service_type='image', endpoint_type='public')
@@ -149,7 +151,9 @@ class ServiceCatalogTest(utils.TestCase):
 
     def test_service_catalog_param_overrides_body_region(self):
         self.AUTH_RESPONSE_BODY['token']['region_name'] = "North"
-        auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
+        # Passing region_name to service catalog is deprecated.
+        with self.deprecations.expect_deprecations_here():
+            auth_ref = access.AccessInfo.factory(None, self.AUTH_RESPONSE_BODY)
         sc = auth_ref.service_catalog
 
         url = sc.url_for(service_type='image')