Make glanceclient accept a session object

To make this work we create a different HTTPClient that extends the
basic keystoneclient Adapter. The Adapter is a standard set of
parameters that all clients should know how to use like region_name and
user_agent. We extend this with the glance specific response
manipulation like loading and sending iterables.

Implements: bp session-objects
Change-Id: Ie8eb4bbf7d1a037099a6d4b272cab70525fbfc85

glanceclient/client.py

@@ -18,31 +18,44 @@ import warnings
 from glanceclient.common import utils
 
 
-def Client(version=None, endpoint=None, *args, **kwargs):
+def Client(version=None, endpoint=None, session=None, *args, **kwargs):
     """Client for the OpenStack Images API.
 
     Generic client for the OpenStack Images API. See version classes
     for specific details.
 
-    :param string version: The version of API to use. Note this is
+    :param string version: The version of API to use.
-                           deprecated and should be passed as part of the URL
+    :param session: A keystoneclient session that should be used for transport.
-                           (http://$HOST:$PORT/v$VERSION_NUMBER).
+    :type session: keystoneclient.session.Session
     """
-    if version is not None:
+    # FIXME(jamielennox): Add a deprecation warning if no session is passed.
-        warnings.warn(("`version` keyword is being deprecated. Please pass the"
+    # Leaving it as an option until we can ensure nothing break when we switch.
-                       " version as part of the URL. "
+    if session:
-                       "http://$HOST:$PORT/v$VERSION_NUMBER"),
+        if endpoint:
-                      DeprecationWarning)
+            kwargs.setdefault('endpoint_override', endpoint)
 
-    endpoint, url_version = utils.strip_version(endpoint)
+            if not version:
+                __, version = utils.strip_version(endpoint)
 
-    if not url_version and not version:
+        if not version:
-        msg = ("Please provide either the version or an url with the form "
+            msg = ("You must provide a client version when using session")
-               "http://$HOST:$PORT/v$VERSION_NUMBER")
+            raise RuntimeError(msg)
-        raise RuntimeError(msg)
 
-    version = int(version or url_version)
+    else:
+        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)
 
-    module = utils.import_versioned_module(version, 'client')
+        endpoint, url_version = utils.strip_version(endpoint)
+        version = version or url_version
+
+        if not version:
+            msg = ("Please provide either the version or an url with the form "
+                   "http://$HOST:$PORT/v$VERSION_NUMBER")
+            raise RuntimeError(msg)
+
+    module = utils.import_versioned_module(int(version), 'client')
     client_class = getattr(module, 'Client')
-    return client_class(endpoint, *args, **kwargs)
+    return client_class(endpoint, *args, session=session, **kwargs)

glanceclient/common/http.py

@@ -17,6 +17,8 @@ import copy
 import logging
 import socket
 
+from keystoneclient import adapter
+from keystoneclient import exceptions as ksc_exc
 from oslo_utils import importutils
 from oslo_utils import netutils
 import requests
@@ -50,7 +52,71 @@ USER_AGENT = 'python-glanceclient'
 CHUNKSIZE = 1024 * 64  # 64kB
 
 
-class HTTPClient(object):
+class _BaseHTTPClient(object):
+
+    @staticmethod
+    def _chunk_body(body):
+        chunk = body
+        while chunk:
+            chunk = body.read(CHUNKSIZE)
+            if chunk == '':
+                break
+            yield chunk
+
+    def _set_common_request_kwargs(self, headers, kwargs):
+        """Handle the common parameters used to send the request."""
+
+        # Default Content-Type is octet-stream
+        content_type = headers.get('Content-Type', 'application/octet-stream')
+
+        # NOTE(jamielennox): remove this later. Managers should pass json= if
+        # they want to send json data.
+        data = kwargs.pop("data", None)
+        if data is not None and not isinstance(data, six.string_types):
+            try:
+                data = json.dumps(data)
+                content_type = 'application/json'
+            except TypeError:
+                # Here we assume it's
+                # a file-like object
+                # and we'll chunk it
+                data = self._chunk_body(data)
+
+        headers['Content-Type'] = content_type
+        kwargs['stream'] = content_type == 'application/octet-stream'
+
+        return data
+
+    def _handle_response(self, resp):
+        if not resp.ok:
+            LOG.debug("Request returned failure status %s." % resp.status_code)
+            raise exc.from_response(resp, resp.content)
+        elif resp.status_code == requests.codes.MULTIPLE_CHOICES:
+            raise exc.from_response(resp)
+
+        content_type = resp.headers.get('Content-Type')
+
+        # Read body into string if it isn't obviously image data
+        if content_type == 'application/octet-stream':
+            # Do not read all response in memory when downloading an image.
+            body_iter = _close_after_stream(resp, CHUNKSIZE)
+        else:
+            content = resp.text
+            if content_type and content_type.startswith('application/json'):
+                # Let's use requests json method, it should take care of
+                # response encoding
+                body_iter = resp.json()
+            else:
+                body_iter = six.StringIO(content)
+                try:
+                    body_iter = json.loads(''.join([c for c in body_iter]))
+                except ValueError:
+                    body_iter = None
+
+        return resp, body_iter
+
+
+class HTTPClient(_BaseHTTPClient):
 
     def __init__(self, endpoint, **kwargs):
         self.endpoint = endpoint
@@ -123,15 +189,16 @@ class HTTPClient(object):
         LOG.debug(msg)
 
     @staticmethod
-    def log_http_response(resp, body=None):
+    def log_http_response(resp):
         status = (resp.raw.version / 10.0, resp.status_code, resp.reason)
         dump = ['\nHTTP/%.1f %s %s' % status]
         headers = resp.headers.items()
         dump.extend(['%s: %s' % safe_header(k, v) for k, v in headers])
         dump.append('')
-        if body:
+        content_type = resp.headers.get('Content-Type')
-            body = encodeutils.safe_decode(body)
+
-            dump.extend([body, ''])
+        if content_type != 'application/octet-stream':
+            dump.extend([resp.text, ''])
         LOG.debug('\n'.join([encodeutils.safe_decode(x, errors='ignore')
                              for x in dump]))
 
@@ -155,37 +222,13 @@ class HTTPClient(object):
         as setting headers and error handling.
         """
         # Copy the kwargs so we can reuse the original in case of redirects
-        headers = kwargs.pop("headers", {})
+        headers = copy.deepcopy(kwargs.pop('headers', {}))
-        headers = headers and copy.deepcopy(headers) or {}
 
         if self.identity_headers:
             for k, v in six.iteritems(self.identity_headers):
                 headers.setdefault(k, v)
 
-        # Default Content-Type is octet-stream
+        data = self._set_common_request_kwargs(headers, kwargs)
-        content_type = headers.get('Content-Type', 'application/octet-stream')
-
-        def chunk_body(body):
-            chunk = body
-            while chunk:
-                chunk = body.read(CHUNKSIZE)
-                if chunk == '':
-                    break
-                yield chunk
-
-        data = kwargs.pop("data", None)
-        if data is not None and not isinstance(data, six.string_types):
-            try:
-                data = json.dumps(data)
-                content_type = 'application/json'
-            except TypeError:
-                # Here we assume it's
-                # a file-like object
-                # and we'll chunk it
-                data = chunk_body(data)
-
-        headers['Content-Type'] = content_type
-        stream = True if content_type == 'application/octet-stream' else False
 
         if osprofiler_web:
             headers.update(osprofiler_web.get_trace_id_headers())
@@ -195,20 +238,20 @@ class HTTPClient(object):
         # complain.
         headers = self.encode_headers(headers)
 
+        if self.endpoint.endswith("/") or url.startswith("/"):
+            conn_url = "%s%s" % (self.endpoint, url)
+        else:
+            conn_url = "%s/%s" % (self.endpoint, url)
+        self.log_curl_request(method, conn_url, headers, data, kwargs)
+
         try:
-            if self.endpoint.endswith("/") or url.startswith("/"):
-                conn_url = "%s%s" % (self.endpoint, url)
-            else:
-                conn_url = "%s/%s" % (self.endpoint, url)
-            self.log_curl_request(method, conn_url, headers, data, kwargs)
             resp = self.session.request(method,
                                         conn_url,
                                         data=data,
-                                        stream=stream,
                                         headers=headers,
                                         **kwargs)
         except requests.exceptions.Timeout as e:
-            message = ("Error communicating with %(endpoint)s %(e)s" %
+            message = ("Error communicating with %(endpoint)s: %(e)s" %
                        dict(url=conn_url, e=e))
             raise exc.InvalidEndpoint(message=message)
         except (requests.exceptions.ConnectionError, ProtocolError) as e:
@@ -225,34 +268,8 @@ class HTTPClient(object):
                        {'endpoint': endpoint, 'e': e})
             raise exc.CommunicationError(message=message)
 
-        if not resp.ok:
+        resp, body_iter = self._handle_response(resp)
-            LOG.debug("Request returned failure status %s." % resp.status_code)
+        self.log_http_response(resp)
-            raise exc.from_response(resp, resp.text)
-        elif resp.status_code == requests.codes.MULTIPLE_CHOICES:
-            raise exc.from_response(resp)
-
-        content_type = resp.headers.get('Content-Type')
-
-        # Read body into string if it isn't obviously image data
-        if content_type == 'application/octet-stream':
-            # Do not read all response in memory when
-            # downloading an image.
-            body_iter = _close_after_stream(resp, CHUNKSIZE)
-            self.log_http_response(resp)
-        else:
-            content = resp.text
-            self.log_http_response(resp, content)
-            if content_type and content_type.startswith('application/json'):
-                # Let's use requests json method,
-                # it should take care of response
-                # encoding
-                body_iter = resp.json()
-            else:
-                body_iter = six.StringIO(content)
-                try:
-                    body_iter = json.loads(''.join([c for c in body_iter]))
-                except ValueError:
-                    body_iter = None
         return resp, body_iter
 
     def head(self, url, **kwargs):
@@ -283,3 +300,45 @@ def _close_after_stream(response, chunk_size):
     # This will return the connection to the HTTPConnectionPool in urllib3
     # and ideally reduce the number of HTTPConnectionPool full warnings.
     response.close()
+
+
+class SessionClient(adapter.Adapter, _BaseHTTPClient):
+
+    def __init__(self, session, **kwargs):
+        kwargs.setdefault('user_agent', USER_AGENT)
+        kwargs.setdefault('service_type', 'image')
+        super(SessionClient, self).__init__(session, **kwargs)
+
+    def request(self, url, method, **kwargs):
+        headers = kwargs.pop('headers', {})
+        kwargs['raise_exc'] = False
+        data = self._set_common_request_kwargs(headers, kwargs)
+
+        try:
+            resp = super(SessionClient, self).request(url,
+                                                      method,
+                                                      headers=headers,
+                                                      data=data,
+                                                      **kwargs)
+        except ksc_exc.RequestTimeout as e:
+            message = ("Error communicating with %(endpoint)s %(e)s" %
+                       dict(url=conn_url, e=e))
+            raise exc.InvalidEndpoint(message=message)
+        except ksc_exc.ConnectionRefused as e:
+            conn_url = self.get_endpoint(auth=kwargs.get('auth'))
+            conn_url = "%s/%s" % (conn_url.rstrip('/'), url.lstrip('/'))
+            message = ("Error finding address for %(url)s: %(e)s" %
+                       dict(url=conn_url, e=e))
+            raise exc.CommunicationError(message=message)
+
+        return self._handle_response(resp)
+
+
+def get_http_client(endpoint=None, session=None, **kwargs):
+    if session:
+        return SessionClient(session, **kwargs)
+    elif endpoint:
+        return HTTPClient(endpoint, **kwargs)
+    else:
+        raise AttributeError('Constructing a client must contain either an '
+                             'endpoint or a session')

glanceclient/common/utils.py

@@ -428,6 +428,14 @@ def safe_header(name, value):
         return name, value
 
 
+def endpoint_version_from_url(endpoint, default_version=None):
+    if endpoint:
+        endpoint, version = strip_version(endpoint)
+        return endpoint, version or default_version
+    else:
+        return None, default_version
+
+
 class IterableWithLength(object):
     def __init__(self, iterable, length):
         self.iterable = iterable

glanceclient/tests/unit/test_http.py

@@ -12,13 +12,17 @@
 #    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 functools
 import json
 
+from keystoneclient.auth import token_endpoint
+from keystoneclient import session
 import mock
 import requests
 from requests_mock.contrib import fixture
 import six
 from six.moves.urllib import parse
+from testscenarios import load_tests_apply_scenarios as load_tests  # noqa
 import testtools
 from testtools import matchers
 import types
@@ -30,15 +34,39 @@ from glanceclient import exc
 from glanceclient.tests import utils
 
 
+def original_only(f):
+    @functools.wraps(f)
+    def wrapper(self, *args, **kwargs):
+        if not hasattr(self.client, 'log_curl_request'):
+            self.skipTest('Skip logging tests for session client')
+
+        return f(self, *args, **kwargs)
+
+
 class TestClient(testtools.TestCase):
 
+    scenarios = [
+        ('httpclient', {'create_client': '_create_http_client'}),
+        ('session', {'create_client': '_create_session_client'})
+    ]
+
+    def _create_http_client(self):
+        return http.HTTPClient(self.endpoint, token=self.token)
+
+    def _create_session_client(self):
+        auth = token_endpoint.Token(self.endpoint, self.token)
+        sess = session.Session(auth=auth)
+        return http.SessionClient(sess)
+
     def setUp(self):
         super(TestClient, self).setUp()
         self.mock = self.useFixture(fixture.Fixture())
 
         self.endpoint = 'http://example.com:9292'
         self.ssl_endpoint = 'https://example.com:9292'
-        self.client = http.HTTPClient(self.endpoint, token=u'abc123')
+        self.token = u'abc123'
+
+        self.client = getattr(self, self.create_client)()
 
     def test_identity_headers_and_token(self):
         identity_headers = {
@@ -140,6 +168,9 @@ class TestClient(testtools.TestCase):
         self.assertEqual(text, resp.text)
 
     def test_headers_encoding(self):
+        if not hasattr(self.client, 'encode_headers'):
+            self.skipTest('Cannot do header encoding check on SessionClient')
+
         value = u'ni\xf1o'
         headers = {"test": value, "none-val": None}
         encoded = self.client.encode_headers(headers)
@@ -206,6 +237,7 @@ class TestClient(testtools.TestCase):
         self.assertTrue(isinstance(body, types.GeneratorType))
         self.assertEqual([data], list(body))
 
+    @original_only
     def test_log_http_response_with_non_ascii_char(self):
         try:
             response = 'Ok'
@@ -216,6 +248,7 @@ class TestClient(testtools.TestCase):
         except UnicodeDecodeError as e:
             self.fail("Unexpected UnicodeDecodeError exception '%s'" % e)
 
+    @original_only
     def test_log_curl_request_with_non_ascii_char(self):
         try:
             headers = {'header1': 'value1\xa5\xa6'}
@@ -225,6 +258,7 @@ class TestClient(testtools.TestCase):
         except UnicodeDecodeError as e:
             self.fail("Unexpected UnicodeDecodeError exception '%s'" % e)
 
+    @original_only
     @mock.patch('glanceclient.common.http.LOG.debug')
     def test_log_curl_request_with_body_and_header(self, mock_log):
         hd_name = 'header1'

glanceclient/v1/client.py

@@ -29,10 +29,9 @@ class Client(object):
                             http requests. (optional)
     """
 
-    def __init__(self, endpoint, **kwargs):
+    def __init__(self, endpoint=None, **kwargs):
         """Initialize a new client for the Images v1 API."""
-        endpoint, version = utils.strip_version(endpoint)
+        endpoint, self.version = utils.endpoint_version_from_url(endpoint, 1.0)
-        self.version = version or 1.0
+        self.http_client = http.get_http_client(endpoint=endpoint, **kwargs)
-        self.http_client = http.HTTPClient(endpoint, **kwargs)
         self.images = ImageManager(self.http_client)
         self.image_members = ImageMemberManager(self.http_client)

glanceclient/v2/client.py

@@ -34,11 +34,9 @@ class Client(object):
                             http requests. (optional)
     """
 
-    def __init__(self, endpoint, **kwargs):
+    def __init__(self, endpoint=None, **kwargs):
-        endpoint, version = utils.strip_version(endpoint)
+        endpoint, self.version = utils.endpoint_version_from_url(endpoint, 2.0)
-        self.version = version or 2.0
+        self.http_client = http.get_http_client(endpoint=endpoint, **kwargs)
-        self.http_client = http.HTTPClient(endpoint, **kwargs)
-
         self.schemas = schemas.Controller(self.http_client)
 
         self.images = images.Controller(self.http_client, self.schemas)

test-requirements.txt

@@ -10,6 +10,7 @@ oslosphinx>=2.5.0  # Apache-2.0
 sphinx>=1.1.2,!=1.2.0,!=1.3b1,<1.3
 testrepository>=0.0.18
 testtools>=0.9.36,!=1.2.0
+testscenarios>=0.4
 fixtures>=0.3.14
 requests-mock>=0.6.0  # Apache-2.0
 tempest-lib>=0.5.0