From 9829d7b6b9f2232bda8039b6db54be1d8885e7a0 Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Mon, 24 Nov 2014 15:12:39 +0100 Subject: [PATCH] Don't require version to create Client instance We currently require a version to always be passed to discover the client version that should be loaded. However, this information is commonly present in the URL instead. The current behavior forces consumers of the library to keep the required version around and/or to strip it themselves from the URL. This patch relaxes that requirement by making the version a keyword and requesting instead an endpoint to be passed. The patch gives priority to the version in the endpoint and falls back to the keyword if the later is not present. Follow-up patches will improve this code making it interact a bit more with the endpoint's catalog. Closes-bug: #1395714 Change-Id: I4ada9e724ac4709429e502b5a006604ca0453f61 --- glanceclient/client.py | 21 ++++++++++++++++++-- glanceclient/common/utils.py | 14 +++++++++++--- glanceclient/shell.py | 27 +++++++++++++++++++++----- glanceclient/v1/client.py | 7 ++++--- glanceclient/v2/client.py | 6 ++++-- tests/{v2 => }/test_client.py | 27 ++++++++++++++++---------- tests/test_shell.py | 4 ++-- tests/v1/test_client.py | 36 ----------------------------------- 8 files changed, 79 insertions(+), 63 deletions(-) rename tests/{v2 => }/test_client.py (56%) delete mode 100644 tests/v1/test_client.py diff --git a/glanceclient/client.py b/glanceclient/client.py index dfebf2f..74afff1 100644 --- a/glanceclient/client.py +++ b/glanceclient/client.py @@ -13,10 +13,27 @@ # License for the specific language governing permissions and limitations # under the License. +import warnings + from glanceclient.common import utils -def Client(version, *args, **kwargs): +def Client(version=None, endpoint=None, *args, **kwargs): + if version is not None: + warnings.warn(("`version` keyword is being deprecated. Please pass the" + " version as part of the URL. " + "http://$HOST:$PORT/v$VERSION_NUMBER"), + DeprecationWarning) + + endpoint, url_version = utils.strip_version(endpoint) + + if not url_version and not version: + msg = ("Please provide either the version or an url with the form " + "http://$HOST:$PORT/v$VERSION_NUMBER") + raise RuntimeError(msg) + + version = int(version or url_version) + module = utils.import_versioned_module(version, 'client') client_class = getattr(module, 'Client') - return client_class(*args, **kwargs) + return client_class(endpoint, *args, **kwargs) diff --git a/glanceclient/common/utils.py b/glanceclient/common/utils.py index d40a704..c0d16a5 100644 --- a/glanceclient/common/utils.py +++ b/glanceclient/common/utils.py @@ -335,14 +335,22 @@ def get_data_file(args): def strip_version(endpoint): """Strip version from the last component of endpoint if present.""" + # NOTE(flaper87): This shouldn't be necessary if + # we make endpoint the first argument. However, we + # can't do that just yet because we need to keep + # backwards compatibility. + if not isinstance(endpoint, six.string_types): + raise ValueError("Expected endpoint") + + version = None # Get rid of trailing '/' if present - if endpoint.endswith('/'): - endpoint = endpoint[:-1] + endpoint = endpoint.rstrip('/') url_bits = endpoint.split('/') # regex to match 'v1' or 'v2.0' etc if re.match('v\d+\.?\d*', url_bits[-1]): + version = float(url_bits[-1].lstrip('v')) endpoint = '/'.join(url_bits[:-1]) - return endpoint + return endpoint, version def print_image(image_obj, max_col_width=None): diff --git a/glanceclient/shell.py b/glanceclient/shell.py index 61c3c72..4460766 100644 --- a/glanceclient/shell.py +++ b/glanceclient/shell.py @@ -248,14 +248,19 @@ class OpenStackImagesShell(object): parser.add_argument('--os-image-url', default=utils.env('OS_IMAGE_URL'), - help='Defaults to env[OS_IMAGE_URL].') + help=('Defaults to env[OS_IMAGE_URL]. ' + 'If the provided image url contains a ' + 'a version number and ' + '`--os-image-api-version` is omitted ' + 'the version of the URL will be picked as ' + 'the image api version to use.')) parser.add_argument('--os_image_url', help=argparse.SUPPRESS) parser.add_argument('--os-image-api-version', default=utils.env('OS_IMAGE_API_VERSION', - default='1'), + default=None), help='Defaults to env[OS_IMAGE_API_VERSION] or 1.') parser.add_argument('--os_image_api_version', @@ -579,10 +584,22 @@ class OpenStackImagesShell(object): parser = self.get_base_parser() (options, args) = parser.parse_known_args(base_argv) - # build available subcommands based on version - api_version = options.os_image_api_version + try: + # NOTE(flaper87): Try to get the version from the + # image-url first. If no version was specified, fallback + # to the api-image-version arg. If both of these fail then + # fallback to the minimum supported one and let keystone + # do the magic. + endpoint = self._get_image_url(options) + endpoint, url_version = utils.strip_version(endpoint) + except ValueError: + # NOTE(flaper87): ValueError is raised if no endpoint is povided + url_version = None - if api_version == '2': + # build available subcommands based on version + api_version = int(options.os_image_api_version or url_version or 1) + + if api_version == 2: self._cache_schemas(options) subcommand_parser = self.get_subcommand_parser(api_version) diff --git a/glanceclient/v1/client.py b/glanceclient/v1/client.py index aeb94a2..668ccfb 100644 --- a/glanceclient/v1/client.py +++ b/glanceclient/v1/client.py @@ -13,7 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. -from glanceclient.common.http import HTTPClient +from glanceclient.common import http from glanceclient.common import utils from glanceclient.v1.image_members import ImageMemberManager from glanceclient.v1.images import ImageManager @@ -31,7 +31,8 @@ class Client(object): def __init__(self, endpoint, *args, **kwargs): """Initialize a new client for the Images v1 API.""" - self.http_client = HTTPClient(utils.strip_version(endpoint), - *args, **kwargs) + endpoint, version = utils.strip_version(endpoint) + self.version = version or 1.0 + self.http_client = http.HTTPClient(endpoint, *args, **kwargs) self.images = ImageManager(self.http_client) self.image_members = ImageMemberManager(self.http_client) diff --git a/glanceclient/v2/client.py b/glanceclient/v2/client.py index f11cb45..8d6f00a 100644 --- a/glanceclient/v2/client.py +++ b/glanceclient/v2/client.py @@ -35,8 +35,10 @@ class Client(object): """ def __init__(self, endpoint, *args, **kwargs): - self.http_client = http.HTTPClient(utils.strip_version(endpoint), - *args, **kwargs) + endpoint, version = utils.strip_version(endpoint) + self.version = version or 2.0 + self.http_client = http.HTTPClient(endpoint, *args, **kwargs) + self.schemas = schemas.Controller(self.http_client) self.images = images.Controller(self.http_client, self.schemas) diff --git a/tests/v2/test_client.py b/tests/test_client.py similarity index 56% rename from tests/v2/test_client.py rename to tests/test_client.py index f775f72..62daaf5 100644 --- a/tests/v2/test_client.py +++ b/tests/test_client.py @@ -1,4 +1,4 @@ -# Copyright 2013 OpenStack LLC. +# Copyright 2014 Red Hat, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -15,25 +15,32 @@ import testtools -from glanceclient.v2 import client +from glanceclient import client +from glanceclient.v1 import client as v1 +from glanceclient.v2 import client as v2 class ClientTest(testtools.TestCase): - def setUp(self): - super(ClientTest, self).setUp() - - def tearDown(self): - super(ClientTest, self).tearDown() + def test_no_endpoint_error(self): + self.assertRaises(ValueError, client.Client, None) def test_endpoint(self): - gc = client.Client("http://example.com") + gc = client.Client(1, "http://example.com") self.assertEqual("http://example.com", gc.http_client.endpoint) + self.assertIsInstance(gc, v1.Client) def test_versioned_endpoint(self): - gc = client.Client("http://example.com/v2") + gc = client.Client(1, "http://example.com/v2") self.assertEqual("http://example.com", gc.http_client.endpoint) + self.assertIsInstance(gc, v1.Client) + + def test_versioned_endpoint_no_version(self): + gc = client.Client(endpoint="http://example.com/v2") + self.assertEqual("http://example.com", gc.http_client.endpoint) + self.assertIsInstance(gc, v2.Client) def test_versioned_endpoint_with_minor_revision(self): - gc = client.Client("http://example.com/v2.1") + gc = client.Client(2.2, "http://example.com/v2.1") self.assertEqual("http://example.com", gc.http_client.endpoint) + self.assertIsInstance(gc, v2.Client) diff --git a/tests/test_shell.py b/tests/test_shell.py index 1a6e5ba..effaedb 100644 --- a/tests/test_shell.py +++ b/tests/test_shell.py @@ -167,7 +167,7 @@ class ShellTest(utils.TestCase): assert v1_client.called (args, kwargs) = v1_client.call_args self.assertEqual('mytoken', kwargs['token']) - self.assertEqual('https://image:1234/v1', args[0]) + self.assertEqual('https://image:1234', args[0]) @mock.patch.object(openstack_shell.OpenStackImagesShell, '_cache_schemas') def test_no_auth_with_token_and_image_url_with_v2(self, @@ -181,7 +181,7 @@ class ShellTest(utils.TestCase): glance_shell = openstack_shell.OpenStackImagesShell() glance_shell.main(args.split()) ((args), kwargs) = v2_client.call_args - self.assertEqual('https://image:1234/v2', args[0]) + self.assertEqual('https://image:1234', args[0]) self.assertEqual('mytoken', kwargs['token']) def _assert_auth_plugin_args(self, mock_auth_plugin): diff --git a/tests/v1/test_client.py b/tests/v1/test_client.py deleted file mode 100644 index e6f8de9..0000000 --- a/tests/v1/test_client.py +++ /dev/null @@ -1,36 +0,0 @@ -# Copyright 2013 OpenStack LLC. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import testtools - -from glanceclient.v1 import client - - -class ClientTest(testtools.TestCase): - - def setUp(self): - super(ClientTest, self).setUp() - - def test_endpoint(self): - gc = client.Client("http://example.com") - self.assertEqual("http://example.com", gc.http_client.endpoint) - - def test_versioned_endpoint(self): - gc = client.Client("http://example.com/v1") - self.assertEqual("http://example.com", gc.http_client.endpoint) - - def test_versioned_endpoint_with_minor_revision(self): - gc = client.Client("http://example.com/v1.1") - self.assertEqual("http://example.com", gc.http_client.endpoint)