From 803eb235d50daad27074198effc98ca536f1550f Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Sun, 26 Jul 2015 07:14:54 -0500 Subject: [PATCH] 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 | 7 ++- keystoneclient/service_catalog.py | 46 +++++++++++++++++-- .../tests/unit/v2_0/test_service_catalog.py | 8 +++- keystoneclient/tests/unit/v3/test_client.py | 1 + .../tests/unit/v3/test_service_catalog.py | 10 ++-- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/keystoneclient/httpclient.py b/keystoneclient/httpclient.py index 85fd1eaae..046d596ea 100644 --- a/keystoneclient/httpclient.py +++ b/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 diff --git a/keystoneclient/service_catalog.py b/keystoneclient/service_catalog.py index 143a6b7c6..b8f4ec205 100644 --- a/keystoneclient/service_catalog.py +++ b/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): diff --git a/keystoneclient/tests/unit/v2_0/test_service_catalog.py b/keystoneclient/tests/unit/v2_0/test_service_catalog.py index fddda6de7..1ea3e052e 100644 --- a/keystoneclient/tests/unit/v2_0/test_service_catalog.py +++ b/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') diff --git a/keystoneclient/tests/unit/v3/test_client.py b/keystoneclient/tests/unit/v3/test_client.py index 6e1c06e11..82e079860 100644 --- a/keystoneclient/tests/unit/v3/test_client.py +++ b/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()) diff --git a/keystoneclient/tests/unit/v3/test_service_catalog.py b/keystoneclient/tests/unit/v3/test_service_catalog.py index 054ad56b3..f1bb08f05 100644 --- a/keystoneclient/tests/unit/v3/test_service_catalog.py +++ b/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')