From 610177a779b95f931356c1e90b05a5bffd2616b3 Mon Sep 17 00:00:00 2001 From: Ravi Jethani Date: Fri, 22 Apr 2016 13:54:49 +0000 Subject: [PATCH] Add request id to returned objects Adding two classes RequestIdProxy and GeneratorProxy derived from wrapt.ObjectProxy to wrap objects returned from the API. GeneratorProxy class is used to wrap generator objects returned by cases like images.list() etc. whereas RequestIdProxy class is used to wrap non-generator object cases like images.create() etc. In all cases the returned object will have the same behavior as the wrapped(original) object. However now the returned objects will have an extra property 'request_ids' which is a list of exactly one request id. For generator cases the request_ids property will be an empty list until the underlying generator is invoked at-least once. Co-Authored-By: Abhishek Kekane Closes-Bug: #1525259 Blueprint: return-request-id-to-caller Change-Id: If8c0e0843270ff718a37ca2697afeb8da22aa3b1 --- glanceclient/common/utils.py | 74 +++++++++ glanceclient/tests/unit/test_utils.py | 50 ++++++ glanceclient/tests/unit/v2/base.py | 119 ++++++++++++++ glanceclient/tests/unit/v2/test_images.py | 141 ++++++++-------- glanceclient/tests/unit/v2/test_members.py | 7 +- .../tests/unit/v2/test_metadefs_namespaces.py | 37 ++--- .../tests/unit/v2/test_metadefs_objects.py | 7 +- .../tests/unit/v2/test_metadefs_properties.py | 8 +- .../unit/v2/test_metadefs_resource_types.py | 12 +- .../tests/unit/v2/test_metadefs_tags.py | 10 +- glanceclient/tests/unit/v2/test_tags.py | 4 +- glanceclient/tests/unit/v2/test_tasks.py | 39 +++-- glanceclient/tests/utils.py | 1 + glanceclient/v2/image_members.py | 15 +- glanceclient/v2/image_tags.py | 8 +- glanceclient/v2/images.py | 79 ++++++--- glanceclient/v2/metadefs.py | 150 +++++++++++++----- glanceclient/v2/tasks.py | 17 +- ...request-id-to-caller-47f4c0a684b1d88e.yaml | 10 ++ requirements.txt | 1 + 20 files changed, 578 insertions(+), 211 deletions(-) create mode 100644 glanceclient/tests/unit/v2/base.py create mode 100644 releasenotes/notes/return-request-id-to-caller-47f4c0a684b1d88e.yaml diff --git a/glanceclient/common/utils.py b/glanceclient/common/utils.py index 93229e64..0221bf47 100644 --- a/glanceclient/common/utils.py +++ b/glanceclient/common/utils.py @@ -36,6 +36,7 @@ else: from oslo_utils import encodeutils from oslo_utils import strutils import prettytable +import wrapt from glanceclient._i18n import _ from glanceclient import exc @@ -472,3 +473,76 @@ class IterableWithLength(object): def __len__(self): return self.length + + +class RequestIdProxy(wrapt.ObjectProxy): + def __init__(self, wrapped): + # `wrapped` is a tuple: (original_obj, response_obj) + super(RequestIdProxy, self).__init__(wrapped[0]) + self._self_wrapped = wrapped[0] + req_id = _extract_request_id(wrapped[1]) + self._self_request_ids = [req_id] + + @property + def request_ids(self): + return self._self_request_ids + + @property + def wrapped(self): + return self._self_wrapped + + +class GeneratorProxy(wrapt.ObjectProxy): + def __init__(self, wrapped): + super(GeneratorProxy, self).__init__(wrapped) + self._self_wrapped = wrapped + self._self_request_ids = [] + + def _set_request_ids(self, resp): + if self._self_request_ids == []: + req_id = _extract_request_id(resp) + self._self_request_ids = [req_id] + + def _next(self): + obj, resp = next(self._self_wrapped) + self._set_request_ids(resp) + return obj + + # Override generator's next method to add + # request id on each iteration + def next(self): + return self._next() + + # For Python 3 compatibility + def __next__(self): + return self._next() + + def __iter__(self): + return self + + @property + def request_ids(self): + return self._self_request_ids + + @property + def wrapped(self): + return self._self_wrapped + + +def add_req_id_to_object(): + @wrapt.decorator + def inner(wrapped, instance, args, kwargs): + return RequestIdProxy(wrapped(*args, **kwargs)) + return inner + + +def add_req_id_to_generator(): + @wrapt.decorator + def inner(wrapped, instance, args, kwargs): + return GeneratorProxy(wrapped(*args, **kwargs)) + return inner + + +def _extract_request_id(resp): + # TODO(rsjethani): Do we need more checks here? + return resp.headers.get('x-openstack-request-id') diff --git a/glanceclient/tests/unit/test_utils.py b/glanceclient/tests/unit/test_utils.py index 272ad594..c0810263 100644 --- a/glanceclient/tests/unit/test_utils.py +++ b/glanceclient/tests/unit/test_utils.py @@ -17,6 +17,7 @@ import sys import mock from oslo_utils import encodeutils +from requests import Response import six # NOTE(jokke): simplified transition to py3, behaves like py2 xrange from six.moves import range @@ -25,6 +26,15 @@ import testtools from glanceclient.common import utils +REQUEST_ID = 'req-1234' + + +def create_response_obj_with_req_id(req_id): + resp = Response() + resp.headers['x-openstack-request-id'] = req_id + return resp + + class TestUtils(testtools.TestCase): def test_make_size_human_readable(self): @@ -178,3 +188,43 @@ class TestUtils(testtools.TestCase): (name, value) = utils.safe_header(sensitive_header, None) self.assertEqual(sensitive_header, name) self.assertIsNone(value) + + def test_generator_proxy(self): + def _test_decorator(): + i = 1 + resp = create_response_obj_with_req_id(REQUEST_ID) + while True: + yield i, resp + i += 1 + + gen_obj = _test_decorator() + proxy = utils.GeneratorProxy(gen_obj) + + # Proxy object should succeed in behaving as the + # wrapped object + self.assertTrue(isinstance(proxy, type(gen_obj))) + + # Initially request_ids should be empty + self.assertEqual([], proxy.request_ids) + + # Only after we have started iterating we should + # see non-empty `request_ids` property + proxy.next() + self.assertEqual([REQUEST_ID], proxy.request_ids) + + # Even after multiple iterations `request_ids` property + # should only contain one request id + proxy.next() + proxy.next() + self.assertEqual(1, len(proxy.request_ids)) + + def test_request_id_proxy(self): + def test_data(val): + resp = create_response_obj_with_req_id(REQUEST_ID) + return val, resp + + # Object of any type except decorator can be passed to test_data + proxy = utils.RequestIdProxy(test_data(11)) + # Verify that proxy object has a property `request_ids` and it is + # a list of one request id + self.assertEqual([REQUEST_ID], proxy.request_ids) diff --git a/glanceclient/tests/unit/v2/base.py b/glanceclient/tests/unit/v2/base.py new file mode 100644 index 00000000..7391595d --- /dev/null +++ b/glanceclient/tests/unit/v2/base.py @@ -0,0 +1,119 @@ +# Copyright 2016 NTT DATA +# +# 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 + + +class BaseController(testtools.TestCase): + def __init__(self, api, schema_api, controller_class): + self.controller = controller_class(api, schema_api) + + def _assertRequestId(self, obj): + self.assertIsNotNone(getattr(obj, 'request_ids', None)) + self.assertEqual(['req-1234'], obj.request_ids) + + def list(self, *args, **kwargs): + gen_obj = self.controller.list(*args, **kwargs) + # For generator cases the request_ids property will be an empty list + # until the underlying generator is invoked at-least once. + resources = list(gen_obj) + if len(resources) > 0: + self._assertRequestId(gen_obj) + else: + # If list is empty that means geneator object has raised + # StopIteration for first iteration and will not contain the + # request_id in it. + self.assertEqual([], gen_obj.request_ids) + + return resources + + def get(self, *args, **kwargs): + resource = self.controller.get(*args, **kwargs) + + self._assertRequestId(resource) + return resource + + def create(self, *args, **kwargs): + resource = self.controller.create(*args, **kwargs) + self._assertRequestId(resource) + return resource + + def create_multiple(self, *args, **kwargs): + tags = self.controller.create_multiple(*args, **kwargs) + actual = [tag.name for tag in tags] + self._assertRequestId(tags) + return actual + + def update(self, *args, **properties): + resource = self.controller.update(*args, **properties) + self._assertRequestId(resource) + return resource + + def delete(self, *args): + resp = self.controller.delete(*args) + self._assertRequestId(resp) + + def delete_all(self, *args): + resp = self.controller.delete_all(*args) + self._assertRequestId(resp) + + def deactivate(self, *args): + resp = self.controller.deactivate(*args) + self._assertRequestId(resp) + + def reactivate(self, *args): + resp = self.controller.reactivate(*args) + self._assertRequestId(resp) + + def upload(self, *args, **kwargs): + resp = self.controller.upload(*args, **kwargs) + self._assertRequestId(resp) + + def data(self, *args, **kwargs): + body = self.controller.data(*args, **kwargs) + self._assertRequestId(body) + return body + + def delete_locations(self, *args): + resp = self.controller.delete_locations(*args) + self._assertRequestId(resp) + + def add_location(self, *args, **kwargs): + resp = self.controller.add_location(*args, **kwargs) + self._assertRequestId(resp) + + def update_location(self, *args, **kwargs): + resp = self.controller.update_location(*args, **kwargs) + self._assertRequestId(resp) + + def associate(self, *args, **kwargs): + resource_types = self.controller.associate(*args, **kwargs) + self._assertRequestId(resource_types) + return resource_types + + def deassociate(self, *args): + resp = self.controller.deassociate(*args) + self._assertRequestId(resp) + + +class BaseResourceTypeController(BaseController): + def __init__(self, api, schema_api, controller_class): + super(BaseResourceTypeController, self).__init__(api, schema_api, + controller_class) + + def get(self, *args, **kwargs): + resource_types = self.controller.get(*args) + names = [rt.name for rt in resource_types] + self._assertRequestId(resource_types) + return names diff --git a/glanceclient/tests/unit/v2/test_images.py b/glanceclient/tests/unit/v2/test_images.py index 0ae38363..5b1670ea 100644 --- a/glanceclient/tests/unit/v2/test_images.py +++ b/glanceclient/tests/unit/v2/test_images.py @@ -18,6 +18,7 @@ import mock import testtools from glanceclient import exc +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import images @@ -532,27 +533,25 @@ class TestController(testtools.TestCase): super(TestController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = images.Controller(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + images.Controller) def test_list_images(self): - # NOTE(bcwaldon):cast to list since the controller returns a generator - images = list(self.controller.list()) + images = self.controller.list() self.assertEqual('3a4560a1-e585-443e-9b39-553b46ec92d1', images[0].id) self.assertEqual('image-1', images[0].name) self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[1].id) self.assertEqual('image-2', images[1].name) def test_list_images_paginated(self): - # NOTE(bcwaldon):cast to list since the controller returns a generator - images = list(self.controller.list(page_size=1)) + images = self.controller.list(page_size=1) self.assertEqual('3a4560a1-e585-443e-9b39-553b46ec92d1', images[0].id) self.assertEqual('image-1', images[0].name) self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[1].id) self.assertEqual('image-2', images[1].name) def test_list_images_paginated_with_limit(self): - # NOTE(bcwaldon):cast to list since the controller returns a generator - images = list(self.controller.list(limit=3, page_size=2)) + images = self.controller.list(limit=3, page_size=2) self.assertEqual('3a4560a1-e585-443e-9b39-553b46ec92d1', images[0].id) self.assertEqual('image-1', images[0].name) self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[1].id) @@ -562,40 +561,40 @@ class TestController(testtools.TestCase): self.assertEqual(3, len(images)) def test_list_images_with_marker(self): - images = list(self.controller.list(limit=1, - marker='3a4560a1-e585-443e-9b39-553b46ec92d1')) + images = self.controller.list( + limit=1, marker='3a4560a1-e585-443e-9b39-553b46ec92d1') self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[0].id) self.assertEqual('image-2', images[0].name) def test_list_images_visibility_public(self): filters = {'filters': {'visibility': 'public'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_PUBLIC_ID, images[0].id) def test_list_images_visibility_private(self): filters = {'filters': {'visibility': 'private'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_PRIVATE_ID, images[0].id) def test_list_images_visibility_shared(self): filters = {'filters': {'visibility': 'shared'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_SHARED_ID, images[0].id) def test_list_images_member_status_rejected(self): filters = {'filters': {'member_status': 'rejected'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_STATUS_REJECTED_ID, images[0].id) def test_list_images_for_owner(self): filters = {'filters': {'owner': _OWNER_ID}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_OWNED_IMAGE_ID, images[0].id) def test_list_images_for_checksum_single_image(self): fake_id = '3a4560a1-e585-443e-9b39-553b46ec92d1' filters = {'filters': {'checksum': _CHKSUM}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(1, len(images)) self.assertEqual('%s' % fake_id, images[0].id) @@ -603,32 +602,32 @@ class TestController(testtools.TestCase): fake_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' fake_id2 = '6f99bf80-2ee6-47cf-acfe-1f1fabb7e810' filters = {'filters': {'checksum': _CHKSUM1}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(2, len(images)) self.assertEqual('%s' % fake_id1, images[0].id) self.assertEqual('%s' % fake_id2, images[1].id) def test_list_images_for_wrong_checksum(self): filters = {'filters': {'checksum': 'wrong'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(0, len(images)) def test_list_images_for_bogus_owner(self): filters = {'filters': {'owner': _BOGUS_ID}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual([], images) def test_list_images_for_bunch_of_filters(self): filters = {'filters': {'owner': _BOGUS_ID, 'visibility': 'shared', 'member_status': 'pending'}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(_EVERYTHING_ID, images[0].id) def test_list_images_filters_encoding(self): filters = {"owner": u"ni\xf1o"} try: - list(self.controller.list(filters=filters)) + self.controller.list(filters=filters) except KeyError: # NOTE(flaper87): It raises KeyError because there's # no fixture supporting this query: @@ -640,7 +639,7 @@ class TestController(testtools.TestCase): def test_list_images_for_tag_single_image(self): img_id = '3a4560a1-e585-443e-9b39-553b46ec92d1' filters = {'filters': {'tag': [_TAG1]}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(1, len(images)) self.assertEqual('%s' % img_id, images[0].id) @@ -648,7 +647,7 @@ class TestController(testtools.TestCase): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' img_id2 = '6f99bf80-2ee6-47cf-acfe-1f1fabb7e810' filters = {'filters': {'tag': [_TAG2]}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[0].id) self.assertEqual('%s' % img_id2, images[1].id) @@ -656,33 +655,32 @@ class TestController(testtools.TestCase): def test_list_images_for_multi_tags(self): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' filters = {'filters': {'tag': [_TAG1, _TAG2]}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(1, len(images)) self.assertEqual('%s' % img_id1, images[0].id) def test_list_images_for_non_existent_tag(self): filters = {'filters': {'tag': ['fake']}} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(0, len(images)) def test_list_images_for_invalid_tag(self): filters = {'filters': {'tag': [[]]}} self.assertRaises(exc.HTTPBadRequest, - list, - self.controller.list(**filters)) + self.controller.list, **filters) def test_list_images_with_single_sort_key(self): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort_key = 'name' - images = list(self.controller.list(sort_key=sort_key)) + images = self.controller.list(sort_key=sort_key) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[0].id) def test_list_with_multiple_sort_keys(self): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort_key = ['name', 'id'] - images = list(self.controller.list(sort_key=sort_key)) + images = self.controller.list(sort_key=sort_key) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[0].id) @@ -690,8 +688,7 @@ class TestController(testtools.TestCase): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort_key = 'id' sort_dir = 'desc' - images = list(self.controller.list(sort_key=sort_key, - sort_dir=sort_dir)) + images = self.controller.list(sort_key=sort_key, sort_dir=sort_dir) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[1].id) @@ -699,8 +696,7 @@ class TestController(testtools.TestCase): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort_key = ['name', 'id'] sort_dir = 'desc' - images = list(self.controller.list(sort_key=sort_key, - sort_dir=sort_dir)) + images = self.controller.list(sort_key=sort_key, sort_dir=sort_dir) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[1].id) @@ -708,61 +704,50 @@ class TestController(testtools.TestCase): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort_key = ['name', 'id'] sort_dir = ['desc', 'asc'] - images = list(self.controller.list(sort_key=sort_key, - sort_dir=sort_dir)) + images = self.controller.list(sort_key=sort_key, sort_dir=sort_dir) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[1].id) def test_list_images_with_new_sorting_syntax(self): img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1' sort = 'name:desc,size:asc' - images = list(self.controller.list(sort=sort)) + images = self.controller.list(sort=sort) self.assertEqual(2, len(images)) self.assertEqual('%s' % img_id1, images[1].id) def test_list_images_sort_dirs_fewer_than_keys(self): sort_key = ['name', 'id', 'created_at'] sort_dir = ['desc', 'asc'] - self.assertRaises(exc.HTTPBadRequest, - list, - self.controller.list( - sort_key=sort_key, - sort_dir=sort_dir)) + self.assertRaises(exc.HTTPBadRequest, self.controller.list, + sort_key=sort_key, sort_dir=sort_dir) def test_list_images_combined_syntax(self): sort_key = ['name', 'id'] sort_dir = ['desc', 'asc'] sort = 'name:asc' self.assertRaises(exc.HTTPBadRequest, - list, - self.controller.list( - sort=sort, - sort_key=sort_key, - sort_dir=sort_dir)) + self.controller.list, sort=sort, sort_key=sort_key, + sort_dir=sort_dir) def test_list_images_new_sorting_syntax_invalid_key(self): sort = 'INVALID:asc' - self.assertRaises(exc.HTTPBadRequest, - list, - self.controller.list( - sort=sort)) + self.assertRaises(exc.HTTPBadRequest, self.controller.list, + sort=sort) def test_list_images_new_sorting_syntax_invalid_direction(self): sort = 'name:INVALID' - self.assertRaises(exc.HTTPBadRequest, - list, - self.controller.list( - sort=sort)) + self.assertRaises(exc.HTTPBadRequest, self.controller.list, + sort=sort) def test_list_images_for_property(self): filters = {'filters': dict([('os_distro', 'NixOS')])} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(1, len(images)) def test_list_images_for_non_existent_property(self): filters = {'filters': dict([('my_little_property', 'cant_be_this_cute')])} - images = list(self.controller.list(**filters)) + images = self.controller.list(**filters) self.assertEqual(0, len(images)) def test_get_image(self): @@ -804,7 +789,7 @@ class TestController(testtools.TestCase): None)] self.assertEqual(expect, self.api.calls) - def reactivate_image(self): + def test_reactivate_image(self): id_image = '87b634c1-f893-33c9-28a9-e5673c99239a' self.controller.reactivate(id_image) expect = [('POST', @@ -868,12 +853,12 @@ class TestController(testtools.TestCase): def test_download_no_data(self): resp = utils.FakeResponse(headers={}, status_code=204) - self.controller.http_client.get = mock.Mock(return_value=(resp, None)) - body = self.controller.data('image_id') - self.assertIsNone(body) + self.controller.controller.http_client.get = mock.Mock( + return_value=(resp, None)) + self.controller.data('image_id') def test_download_forbidden(self): - self.controller.http_client.get = mock.Mock( + self.controller.controller.http_client.get = mock.Mock( side_effect=exc.HTTPForbidden()) try: self.controller.data('image_id') @@ -893,7 +878,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -912,7 +898,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -931,7 +918,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -953,7 +941,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -974,7 +963,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -999,7 +989,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -1016,7 +1007,8 @@ class TestController(testtools.TestCase): expect = [ ('GET', '/v2/images/%s' % image_id, {}, None), ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body), - ('GET', '/v2/images/%s' % image_id, {}, None), + ('GET', '/v2/images/%s' % image_id, + {'x-openstack-request-id': 'req-1234'}, None), ] self.assertEqual(expect, self.api.calls) self.assertEqual(image_id, image.id) @@ -1040,8 +1032,9 @@ class TestController(testtools.TestCase): image_id, url, meta) self.assertIn(estr, str(e)) - def _empty_get(self, image_id): - return ('GET', '/v2/images/%s' % image_id, {}, None) + def _empty_get(self, image_id, headers=None): + return ('GET', '/v2/images/%s' % image_id, + headers or {}, None) def _patch_req(self, image_id, patch_body): c_type = 'application/openstack-images-v2.1-json-patch' @@ -1055,9 +1048,11 @@ class TestController(testtools.TestCase): image_id = 'a2b83adc-888e-11e3-8872-78acc0b951d8' new_loc = {'url': 'http://spam.com/', 'metadata': {'spam': 'ham'}} add_patch = {'path': '/locations/-', 'value': new_loc, 'op': 'add'} + headers = {'x-openstack-request-id': 'req-1234'} self.controller.add_location(image_id, **new_loc) self.assertEqual([self._patch_req(image_id, [add_patch]), - self._empty_get(image_id)], self.api.calls) + self._empty_get(image_id, headers=headers)], + self.api.calls) @mock.patch.object(images.Controller, '_send_image_update_request', side_effect=exc.HTTPBadRequest) @@ -1092,6 +1087,7 @@ class TestController(testtools.TestCase): def test_update_location(self): image_id = 'a2b83adc-888e-11e3-8872-78acc0b951d8' new_loc = {'url': 'http://foo.com/', 'metadata': {'spam': 'ham'}} + headers = {'x-openstack-request-id': 'req-1234'} fixture_idx = '/v2/images/%s' % (image_id) orig_locations = data_fixtures[fixture_idx]['GET'][1]['locations'] loc_map = dict([(l['url'], l) for l in orig_locations]) @@ -1101,12 +1097,13 @@ class TestController(testtools.TestCase): self.controller.update_location(image_id, **new_loc) self.assertEqual([self._empty_get(image_id), self._patch_req(image_id, mod_patch), - self._empty_get(image_id)], + self._empty_get(image_id, headers=headers)], self.api.calls) def test_update_tags(self): image_id = 'a2b83adc-888e-11e3-8872-78acc0b951d8' tag_map = {'tags': ['tag01', 'tag02', 'tag03']} + headers = {'x-openstack-request-id': 'req-1234'} image = self.controller.update(image_id, **tag_map) @@ -1115,7 +1112,7 @@ class TestController(testtools.TestCase): expected = [ self._empty_get(image_id), self._patch_req(image_id, expected_body), - self._empty_get(image_id) + self._empty_get(image_id, headers=headers) ] self.assertEqual(expected, self.api.calls) self.assertEqual(image_id, image.id) diff --git a/glanceclient/tests/unit/v2/test_members.py b/glanceclient/tests/unit/v2/test_members.py index be378f91..7048a971 100644 --- a/glanceclient/tests/unit/v2/test_members.py +++ b/glanceclient/tests/unit/v2/test_members.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import image_members @@ -80,12 +81,12 @@ class TestController(testtools.TestCase): super(TestController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = image_members.Controller(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + image_members.Controller) def test_list_image_members(self): image_id = IMAGE - # NOTE(iccha): cast to list since the controller returns a generator - image_members = list(self.controller.list(image_id)) + image_members = self.controller.list(image_id) self.assertEqual(IMAGE, image_members[0].image_id) self.assertEqual(MEMBER, image_members[0].member_id) diff --git a/glanceclient/tests/unit/v2/test_metadefs_namespaces.py b/glanceclient/tests/unit/v2/test_metadefs_namespaces.py index afadda81..1c19d8bf 100644 --- a/glanceclient/tests/unit/v2/test_metadefs_namespaces.py +++ b/glanceclient/tests/unit/v2/test_metadefs_namespaces.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import metadefs @@ -542,83 +543,79 @@ class TestNamespaceController(testtools.TestCase): super(TestNamespaceController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = metadefs.NamespaceController(self.api, - self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + metadefs.NamespaceController) def test_list_namespaces(self): - namespaces = list(self.controller.list()) - + namespaces = self.controller.list() self.assertEqual(2, len(namespaces)) self.assertEqual(NAMESPACE1, namespaces[0]['namespace']) self.assertEqual(NAMESPACE2, namespaces[1]['namespace']) def test_list_namespaces_paginate(self): - namespaces = list(self.controller.list(page_size=1)) - + namespaces = self.controller.list(page_size=1) self.assertEqual(2, len(namespaces)) self.assertEqual(NAMESPACE7, namespaces[0]['namespace']) self.assertEqual(NAMESPACE8, namespaces[1]['namespace']) def test_list_with_limit_greater_than_page_size(self): - namespaces = list(self.controller.list(page_size=1, limit=2)) + namespaces = self.controller.list(page_size=1, limit=2) self.assertEqual(2, len(namespaces)) self.assertEqual(NAMESPACE7, namespaces[0]['namespace']) self.assertEqual(NAMESPACE8, namespaces[1]['namespace']) def test_list_with_marker(self): - namespaces = list(self.controller.list(marker=NAMESPACE6, page_size=2)) + namespaces = self.controller.list(marker=NAMESPACE6, page_size=2) self.assertEqual(2, len(namespaces)) self.assertEqual(NAMESPACE7, namespaces[0]['namespace']) self.assertEqual(NAMESPACE8, namespaces[1]['namespace']) def test_list_with_sort_dir(self): - namespaces = list(self.controller.list(sort_dir='asc', limit=1)) + namespaces = self.controller.list(sort_dir='asc', limit=1) self.assertEqual(1, len(namespaces)) self.assertEqual(NAMESPACE1, namespaces[0]['namespace']) def test_list_with_sort_dir_invalid(self): # NOTE(TravT): The clients work by returning an iterator. # Invoking the iterator is what actually executes the logic. - ns_iterator = self.controller.list(sort_dir='foo') - self.assertRaises(ValueError, next, ns_iterator) + self.assertRaises(ValueError, self.controller.list, sort_dir='foo') def test_list_with_sort_key(self): - namespaces = list(self.controller.list(sort_key='created_at', limit=1)) + namespaces = self.controller.list(sort_key='created_at', limit=1) self.assertEqual(1, len(namespaces)) self.assertEqual(NAMESPACE1, namespaces[0]['namespace']) def test_list_with_sort_key_invalid(self): # NOTE(TravT): The clients work by returning an iterator. # Invoking the iterator is what actually executes the logic. - ns_iterator = self.controller.list(sort_key='foo') - self.assertRaises(ValueError, next, ns_iterator) + self.assertRaises(ValueError, self.controller.list, sort_key='foo') def test_list_namespaces_with_one_resource_type_filter(self): - namespaces = list(self.controller.list( + namespaces = self.controller.list( filters={ 'resource_types': [RESOURCE_TYPE1] } - )) + ) self.assertEqual(1, len(namespaces)) self.assertEqual(NAMESPACE3, namespaces[0]['namespace']) def test_list_namespaces_with_multiple_resource_types_filter(self): - namespaces = list(self.controller.list( + namespaces = self.controller.list( filters={ 'resource_types': [RESOURCE_TYPE1, RESOURCE_TYPE2] } - )) + ) self.assertEqual(1, len(namespaces)) self.assertEqual(NAMESPACE4, namespaces[0]['namespace']) def test_list_namespaces_with_visibility_filter(self): - namespaces = list(self.controller.list( + namespaces = self.controller.list( filters={ 'visibility': 'private' } - )) + ) self.assertEqual(1, len(namespaces)) self.assertEqual(NAMESPACE5, namespaces[0]['namespace']) diff --git a/glanceclient/tests/unit/v2/test_metadefs_objects.py b/glanceclient/tests/unit/v2/test_metadefs_objects.py index f554fcbb..509f7550 100644 --- a/glanceclient/tests/unit/v2/test_metadefs_objects.py +++ b/glanceclient/tests/unit/v2/test_metadefs_objects.py @@ -16,6 +16,7 @@ import six import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import metadefs @@ -263,11 +264,11 @@ class TestObjectController(testtools.TestCase): super(TestObjectController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = metadefs.ObjectController(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + metadefs.ObjectController) def test_list_object(self): - objects = list(self.controller.list(NAMESPACE1)) - + objects = self.controller.list(NAMESPACE1) actual = [obj.name for obj in objects] self.assertEqual([OBJECT1, OBJECT2], actual) diff --git a/glanceclient/tests/unit/v2/test_metadefs_properties.py b/glanceclient/tests/unit/v2/test_metadefs_properties.py index 11165b96..68c37337 100644 --- a/glanceclient/tests/unit/v2/test_metadefs_properties.py +++ b/glanceclient/tests/unit/v2/test_metadefs_properties.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import metadefs @@ -238,12 +239,11 @@ class TestPropertyController(testtools.TestCase): super(TestPropertyController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = metadefs.PropertyController(self.api, - self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + metadefs.PropertyController) def test_list_property(self): - properties = list(self.controller.list(NAMESPACE1)) - + properties = self.controller.list(NAMESPACE1) actual = [prop.name for prop in properties] self.assertEqual(sorted([PROPERTY1, PROPERTY2]), sorted(actual)) diff --git a/glanceclient/tests/unit/v2/test_metadefs_resource_types.py b/glanceclient/tests/unit/v2/test_metadefs_resource_types.py index 7893e963..0735bb54 100644 --- a/glanceclient/tests/unit/v2/test_metadefs_resource_types.py +++ b/glanceclient/tests/unit/v2/test_metadefs_resource_types.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import metadefs @@ -151,18 +152,17 @@ class TestResoureTypeController(testtools.TestCase): super(TestResoureTypeController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = metadefs.ResourceTypeController(self.api, - self.schema_api) + self.controller = base.BaseResourceTypeController( + self.api, self.schema_api, metadefs.ResourceTypeController) def test_list_resource_types(self): - resource_types = list(self.controller.list()) + resource_types = self.controller.list() names = [rt.name for rt in resource_types] self.assertEqual([RESOURCE_TYPE1, RESOURCE_TYPE2], names) def test_get_resource_types(self): - resource_types = list(self.controller.get(NAMESPACE1)) - names = [rt.name for rt in resource_types] - self.assertEqual([RESOURCE_TYPE3, RESOURCE_TYPE4], names) + resource_types = self.controller.get(NAMESPACE1) + self.assertEqual([RESOURCE_TYPE3, RESOURCE_TYPE4], resource_types) def test_associate_resource_types(self): resource_types = self.controller.associate(NAMESPACE1, diff --git a/glanceclient/tests/unit/v2/test_metadefs_tags.py b/glanceclient/tests/unit/v2/test_metadefs_tags.py index 1df53b6a..936e13e7 100644 --- a/glanceclient/tests/unit/v2/test_metadefs_tags.py +++ b/glanceclient/tests/unit/v2/test_metadefs_tags.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import metadefs @@ -134,11 +135,11 @@ class TestTagController(testtools.TestCase): super(TestTagController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = metadefs.TagController(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + metadefs.TagController) def test_list_tag(self): - tags = list(self.controller.list(NAMESPACE1)) - + tags = self.controller.list(NAMESPACE1) actual = [tag.name for tag in tags] self.assertEqual([TAG1, TAG2], actual) @@ -155,8 +156,7 @@ class TestTagController(testtools.TestCase): 'tags': [TAGNEW2, TAGNEW3] } tags = self.controller.create_multiple(NAMESPACE1, **properties) - actual = [tag.name for tag in tags] - self.assertEqual([TAGNEW2, TAGNEW3], actual) + self.assertEqual([TAGNEW2, TAGNEW3], tags) def test_update_tag(self): properties = { diff --git a/glanceclient/tests/unit/v2/test_tags.py b/glanceclient/tests/unit/v2/test_tags.py index 790080ed..9a6aec55 100644 --- a/glanceclient/tests/unit/v2/test_tags.py +++ b/glanceclient/tests/unit/v2/test_tags.py @@ -15,6 +15,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import image_tags @@ -54,7 +55,8 @@ class TestController(testtools.TestCase): super(TestController, self).setUp() self.api = utils.FakeAPI(data_fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = image_tags.Controller(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + image_tags.Controller) def test_update_image_tag(self): image_id = IMAGE diff --git a/glanceclient/tests/unit/v2/test_tasks.py b/glanceclient/tests/unit/v2/test_tasks.py index 860b569b..2da50f20 100644 --- a/glanceclient/tests/unit/v2/test_tasks.py +++ b/glanceclient/tests/unit/v2/test_tasks.py @@ -16,6 +16,7 @@ import testtools +from glanceclient.tests.unit.v2 import base from glanceclient.tests import utils from glanceclient.v2 import tasks @@ -252,11 +253,11 @@ class TestController(testtools.TestCase): super(TestController, self).setUp() self.api = utils.FakeAPI(fixtures) self.schema_api = utils.FakeSchemaAPI(schema_fixtures) - self.controller = tasks.Controller(self.api, self.schema_api) + self.controller = base.BaseController(self.api, self.schema_api, + tasks.Controller) def test_list_tasks(self): - # NOTE(flwang): cast to list since the controller returns a generator - tasks = list(self.controller.list()) + tasks = self.controller.list() self.assertEqual(_PENDING_ID, tasks[0].id) self.assertEqual('import', tasks[0].type) self.assertEqual('pending', tasks[0].status) @@ -265,8 +266,7 @@ class TestController(testtools.TestCase): self.assertEqual('processing', tasks[1].status) def test_list_tasks_paginated(self): - # NOTE(flwang): cast to list since the controller returns a generator - tasks = list(self.controller.list(page_size=1)) + tasks = self.controller.list(page_size=1) self.assertEqual(_PENDING_ID, tasks[0].id) self.assertEqual('import', tasks[0].type) self.assertEqual(_PROCESSING_ID, tasks[1].id) @@ -274,38 +274,38 @@ class TestController(testtools.TestCase): def test_list_tasks_with_status(self): filters = {'filters': {'status': 'processing'}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(_OWNED_TASK_ID, tasks[0].id) def test_list_tasks_with_wrong_status(self): filters = {'filters': {'status': 'fake'}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(0, len(tasks)) def test_list_tasks_with_type(self): filters = {'filters': {'type': 'import'}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(_OWNED_TASK_ID, tasks[0].id) def test_list_tasks_with_wrong_type(self): filters = {'filters': {'type': 'fake'}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(0, len(tasks)) def test_list_tasks_for_owner(self): filters = {'filters': {'owner': _OWNER_ID}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(_OWNED_TASK_ID, tasks[0].id) def test_list_tasks_for_fake_owner(self): filters = {'filters': {'owner': _FAKE_OWNER_ID}} - tasks = list(self.controller.list(**filters)) + tasks = self.controller.list(**filters) self.assertEqual(tasks, []) def test_list_tasks_filters_encoding(self): filters = {"owner": u"ni\xf1o"} try: - list(self.controller.list(filters=filters)) + self.controller.list(filters=filters) except KeyError: # NOTE(flaper87): It raises KeyError because there's # no fixture supporting this query: @@ -316,34 +316,33 @@ class TestController(testtools.TestCase): self.assertEqual(b"ni\xc3\xb1o", filters["owner"]) def test_list_tasks_with_marker(self): - tasks = list(self.controller.list(marker=_PENDING_ID, page_size=1)) + tasks = self.controller.list(marker=_PENDING_ID, page_size=1) self.assertEqual(1, len(tasks)) self.assertEqual(_PROCESSING_ID, tasks[0]['id']) def test_list_tasks_with_single_sort_key(self): - tasks = list(self.controller.list(sort_key='type')) + tasks = self.controller.list(sort_key='type') self.assertEqual(2, len(tasks)) self.assertEqual(_PENDING_ID, tasks[0].id) def test_list_tasks_with_invalid_sort_key(self): self.assertRaises(ValueError, - list, - self.controller.list(sort_key='invalid')) + self.controller.list, sort_key='invalid') def test_list_tasks_with_desc_sort_dir(self): - tasks = list(self.controller.list(sort_key='id', sort_dir='desc')) + tasks = self.controller.list(sort_key='id', sort_dir='desc') self.assertEqual(2, len(tasks)) self.assertEqual(_PENDING_ID, tasks[1].id) def test_list_tasks_with_asc_sort_dir(self): - tasks = list(self.controller.list(sort_key='id', sort_dir='asc')) + tasks = self.controller.list(sort_key='id', sort_dir='asc') self.assertEqual(2, len(tasks)) self.assertEqual(_PENDING_ID, tasks[0].id) def test_list_tasks_with_invalid_sort_dir(self): self.assertRaises(ValueError, - list, - self.controller.list(sort_dir='invalid')) + self.controller.list, + sort_dir='invalid') def test_get_task(self): task = self.controller.get(_PENDING_ID) diff --git a/glanceclient/tests/utils.py b/glanceclient/tests/utils.py index 6b03f312..3deddb1c 100644 --- a/glanceclient/tests/utils.py +++ b/glanceclient/tests/utils.py @@ -113,6 +113,7 @@ class FakeResponse(object): self.reason = reason self.version = version self.headers = headers + self.headers['x-openstack-request-id'] = 'req-1234' self.status_code = status_code self.raw = RawRequest(headers, body=body, reason=reason, version=version, status=status_code) diff --git a/glanceclient/v2/image_members.py b/glanceclient/v2/image_members.py index 5d07b9b5..780519a2 100644 --- a/glanceclient/v2/image_members.py +++ b/glanceclient/v2/image_members.py @@ -32,24 +32,29 @@ class Controller(object): schema = self.schema_client.get('member') return warlock.model_factory(schema.raw(), schemas.SchemaBasedModel) + @utils.add_req_id_to_generator() def list(self, image_id): url = '/v2/images/%s/members' % image_id resp, body = self.http_client.get(url) for member in body['members']: - yield self.model(member) + yield self.model(member), resp + @utils.add_req_id_to_object() def delete(self, image_id, member_id): - self.http_client.delete('/v2/images/%s/members/%s' % - (image_id, member_id)) + resp, body = self.http_client.delete('/v2/images/%s/members/%s' % + (image_id, member_id)) + return (resp, body), resp + @utils.add_req_id_to_object() def update(self, image_id, member_id, member_status): url = '/v2/images/%s/members/%s' % (image_id, member_id) body = {'status': member_status} resp, updated_member = self.http_client.put(url, data=body) - return self.model(updated_member) + return self.model(updated_member), resp + @utils.add_req_id_to_object() def create(self, image_id, member_id): url = '/v2/images/%s/members' % image_id body = {'member': member_id} resp, created_member = self.http_client.post(url, data=body) - return self.model(created_member) + return self.model(created_member), resp diff --git a/glanceclient/v2/image_tags.py b/glanceclient/v2/image_tags.py index deebce2c..1016da5e 100644 --- a/glanceclient/v2/image_tags.py +++ b/glanceclient/v2/image_tags.py @@ -30,6 +30,7 @@ class Controller(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def update(self, image_id, tag_value): """Update an image with the given tag. @@ -37,8 +38,10 @@ class Controller(object): :param tag_value: value of the tag. """ url = '/v2/images/%s/tags/%s' % (image_id, tag_value) - self.http_client.put(url) + resp, body = self.http_client.put(url) + return (resp, body), resp + @utils.add_req_id_to_object() def delete(self, image_id, tag_value): """Delete the tag associated with the given image. @@ -46,4 +49,5 @@ class Controller(object): :param tag_value: tag value to be deleted. """ url = '/v2/images/%s/tags/%s' % (image_id, tag_value) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index f69fed54..38895b8a 100644 --- a/glanceclient/v2/images.py +++ b/glanceclient/v2/images.py @@ -81,6 +81,7 @@ class Controller(object): raise exc.HTTPBadRequest(msg) return sort + @utils.add_req_id_to_generator() def list(self, **kwargs): """Retrieve a listing of Image objects. @@ -97,6 +98,7 @@ class Controller(object): def paginate(url, page_size, limit=None): next_url = url + req_id_hdr = {} while True: if limit and page_size > limit: @@ -105,7 +107,12 @@ class Controller(object): next_url = next_url.replace("limit=%s" % page_size, "limit=%s" % limit) - resp, body = self.http_client.get(next_url) + resp, body = self.http_client.get(next_url, headers=req_id_hdr) + # NOTE(rsjethani): Store curent request id so that it can be + # used in subsequent requests. Refer bug #1525259 + req_id_hdr['x-openstack-request-id'] = \ + utils._extract_request_id(resp) + for image in body['images']: # NOTE(bcwaldon): remove 'self' for now until we have # an elegant way to pass it into the model constructor @@ -114,7 +121,7 @@ class Controller(object): # We do not validate the model when listing. # This prevents side-effects of injecting invalid # schema values via v1. - yield self.unvalidated_model(**image) + yield self.unvalidated_model(**image), resp if limit: limit -= 1 if limit <= 0: @@ -173,17 +180,23 @@ class Controller(object): if isinstance(kwargs.get('marker'), six.string_types): url = '%s&marker=%s' % (url, kwargs['marker']) - for image in paginate(url, page_size, limit): - yield image + for image, resp in paginate(url, page_size, limit): + yield image, resp - def get(self, image_id): + @utils.add_req_id_to_object() + def _get(self, image_id, header=None): url = '/v2/images/%s' % image_id - resp, body = self.http_client.get(url) + header = header or {} + resp, body = self.http_client.get(url, headers=header) # NOTE(bcwaldon): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict body.pop('self', None) - return self.unvalidated_model(**body) + return self.unvalidated_model(**body), resp + def get(self, image_id): + return self._get(image_id) + + @utils.add_req_id_to_object() def data(self, image_id, do_checksum=True): """Retrieve data of an image. @@ -194,7 +207,7 @@ class Controller(object): url = '/v2/images/%s/file' % image_id resp, body = self.http_client.get(url) if resp.status_code == codes.no_content: - return None + return None, resp checksum = resp.headers.get('content-md5', None) content_length = int(resp.headers.get('content-length', 0)) @@ -202,8 +215,9 @@ class Controller(object): if do_checksum and checksum is not None: body = utils.integrity_iter(body, checksum) - return utils.IterableWithLength(body, content_length) + return utils.IterableWithLength(body, content_length), resp + @utils.add_req_id_to_object() def upload(self, image_id, image_data, image_size=None): """Upload the data for an image. @@ -214,13 +228,17 @@ class Controller(object): url = '/v2/images/%s/file' % image_id hdrs = {'Content-Type': 'application/octet-stream'} body = image_data - self.http_client.put(url, headers=hdrs, data=body) + resp, body = self.http_client.put(url, headers=hdrs, data=body) + return (resp, body), resp + @utils.add_req_id_to_object() def delete(self, image_id): """Delete an image.""" url = '/v2/images/%s' % image_id - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp + @utils.add_req_id_to_object() def create(self, **kwargs): """Create an image.""" url = '/v2/images' @@ -236,17 +254,21 @@ class Controller(object): # NOTE(esheffield): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_object() def deactivate(self, image_id): """Deactivate an image.""" url = '/v2/images/%s/actions/deactivate' % image_id - return self.http_client.post(url) + resp, body = self.http_client.post(url) + return (resp, body), resp + @utils.add_req_id_to_object() def reactivate(self, image_id): """Reactivate an image.""" url = '/v2/images/%s/actions/reactivate' % image_id - return self.http_client.post(url) + resp, body = self.http_client.post(url) + return (resp, body), resp def update(self, image_id, remove_props=None, **kwargs): """Update attributes of an image. @@ -276,12 +298,16 @@ class Controller(object): url = '/v2/images/%s' % image_id hdrs = {'Content-Type': 'application/openstack-images-v2.1-json-patch'} - self.http_client.patch(url, headers=hdrs, data=image.patch) + resp, _ = self.http_client.patch(url, headers=hdrs, data=image.patch) + # Get request id from `patch` request so it can be passed to the + # following `get` call + req_id_hdr = { + 'x-openstack-request-id': utils._extract_request_id(resp)} # NOTE(bcwaldon): calling image.patch doesn't clear the changes, so # we need to fetch the image again to get a clean history. This is # an obvious optimization for warlock - return self.get(image_id) + return self._get(image_id, req_id_hdr) def _get_image_with_locations_or_fail(self, image_id): image = self.get(image_id) @@ -290,10 +316,13 @@ class Controller(object): 'API access to image locations') return image + @utils.add_req_id_to_object() def _send_image_update_request(self, image_id, patch_body): url = '/v2/images/%s' % image_id hdrs = {'Content-Type': 'application/openstack-images-v2.1-json-patch'} - self.http_client.patch(url, headers=hdrs, data=json.dumps(patch_body)) + resp, body = self.http_client.patch(url, headers=hdrs, + data=json.dumps(patch_body)) + return (resp, body), resp def add_location(self, image_id, url, metadata): """Add a new location entry to an image's list of locations. @@ -308,8 +337,11 @@ class Controller(object): """ add_patch = [{'op': 'add', 'path': '/locations/-', 'value': {'url': url, 'metadata': metadata}}] - self._send_image_update_request(image_id, add_patch) - return self.get(image_id) + response = self._send_image_update_request(image_id, add_patch) + # Get request id from the above update request and pass the same to + # following get request + req_id_hdr = {'x-openstack-request-id': response.request_ids[0]} + return self._get(image_id, req_id_hdr) def delete_locations(self, image_id, url_set): """Remove one or more location entries of an image. @@ -332,7 +364,7 @@ class Controller(object): url_indices.sort(reverse=True) patches = [{'op': 'remove', 'path': '/locations/%s' % url_idx} for url_idx in url_indices] - self._send_image_update_request(image_id, patches) + return self._send_image_update_request(image_id, patches) def update_location(self, image_id, url, metadata): """Update an existing location entry in an image's list of locations. @@ -359,6 +391,9 @@ class Controller(object): patches = [{'op': 'replace', 'path': '/locations', 'value': list(url_map.values())}] - self._send_image_update_request(image_id, patches) + response = self._send_image_update_request(image_id, patches) + # Get request id from the above update request and pass the same to + # following get request + req_id_hdr = {'x-openstack-request-id': response.request_ids[0]} - return self.get(image_id) + return self._get(image_id, req_id_hdr) diff --git a/glanceclient/v2/metadefs.py b/glanceclient/v2/metadefs.py index 4bee2243..316ee2a5 100644 --- a/glanceclient/v2/metadefs.py +++ b/glanceclient/v2/metadefs.py @@ -37,6 +37,7 @@ class NamespaceController(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def create(self, **kwargs): """Create a namespace. @@ -50,7 +51,7 @@ class NamespaceController(object): resp, body = self.http_client.post(url, data=namespace) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp def update(self, namespace_name, **kwargs): """Update a namespace. @@ -72,23 +73,34 @@ class NamespaceController(object): del namespace[elem] url = '/v2/metadefs/namespaces/{0}'.format(namespace_name) - self.http_client.put(url, data=namespace) - - return self.get(namespace.namespace) + # Pass the original wrapped value to http client. + resp, _ = self.http_client.put(url, data=namespace.wrapped) + # Get request id from `put` request so it can be passed to the + # following `get` call + req_id_hdr = { + 'x-openstack-request-id': utils._extract_request_id(resp) + } + return self._get(namespace.namespace, header=req_id_hdr) def get(self, namespace, **kwargs): + return self._get(namespace, **kwargs) + + @utils.add_req_id_to_object() + def _get(self, namespace, header=None, **kwargs): """Get one namespace.""" query_params = parse.urlencode(kwargs) if kwargs: query_params = '?%s' % query_params url = '/v2/metadefs/namespaces/{0}{1}'.format(namespace, query_params) - resp, body = self.http_client.get(url) + header = header or {} + resp, body = self.http_client.get(url, headers=header) # NOTE(bcwaldon): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_generator() def list(self, **kwargs): """Retrieve a listing of Namespace objects. @@ -117,7 +129,7 @@ class NamespaceController(object): # an elegant way to pass it into the model constructor # without conflict. namespace.pop('self', None) - yield self.model(**namespace) + yield self.model(**namespace), resp # NOTE(zhiyan): In order to resolve the performance issue # of JSON schema validation for image listing case, we # don't validate each image entry but do it only on first @@ -132,8 +144,8 @@ class NamespaceController(object): except KeyError: return else: - for namespace in paginate(next_url): - yield namespace + for namespace, resp in paginate(next_url): + yield namespace, resp filters = kwargs.get('filters', {}) filters = {} if filters is None else filters @@ -170,13 +182,15 @@ class NamespaceController(object): url = '/v2/metadefs/namespaces?%s' % parse.urlencode(filters) - for namespace in paginate(url): - yield namespace + for namespace, resp in paginate(url): + yield namespace, resp + @utils.add_req_id_to_object() def delete(self, namespace): """Delete a namespace.""" url = '/v2/metadefs/namespaces/{0}'.format(namespace) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp class ResourceTypeController(object): @@ -190,6 +204,7 @@ class ResourceTypeController(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def associate(self, namespace, **kwargs): """Associate a resource type with a namespace.""" try: @@ -201,14 +216,17 @@ class ResourceTypeController(object): res_type) resp, body = self.http_client.post(url, data=res_type) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_object() def deassociate(self, namespace, resource): """Deassociate a resource type with a namespace.""" url = '/v2/metadefs/namespaces/{0}/resource_types/{1}'. \ format(namespace, resource) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp + @utils.add_req_id_to_generator() def list(self): """Retrieve a listing of available resource types. @@ -218,14 +236,15 @@ class ResourceTypeController(object): url = '/v2/metadefs/resource_types' resp, body = self.http_client.get(url) for resource_type in body['resource_types']: - yield self.model(**resource_type) + yield self.model(**resource_type), resp + @utils.add_req_id_to_generator() def get(self, namespace): url = '/v2/metadefs/namespaces/{0}/resource_types'.format(namespace) resp, body = self.http_client.get(url) body.pop('self', None) for resource_type in body['resource_type_associations']: - yield self.model(**resource_type) + yield self.model(**resource_type), resp class PropertyController(object): @@ -239,6 +258,7 @@ class PropertyController(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def create(self, namespace, **kwargs): """Create a property. @@ -254,7 +274,7 @@ class PropertyController(object): resp, body = self.http_client.post(url, data=prop) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp def update(self, namespace, prop_name, **kwargs): """Update a property. @@ -272,18 +292,29 @@ class PropertyController(object): url = '/v2/metadefs/namespaces/{0}/properties/{1}'.format(namespace, prop_name) - self.http_client.put(url, data=prop) + # Pass the original wrapped value to http client. + resp, _ = self.http_client.put(url, data=prop.wrapped) + # Get request id from `put` request so it can be passed to the + # following `get` call + req_id_hdr = { + 'x-openstack-request-id': utils._extract_request_id(resp)} - return self.get(namespace, prop.name) + return self._get(namespace, prop.name, req_id_hdr) def get(self, namespace, prop_name): + return self._get(namespace, prop_name) + + @utils.add_req_id_to_object() + def _get(self, namespace, prop_name, header=None): url = '/v2/metadefs/namespaces/{0}/properties/{1}'.format(namespace, prop_name) - resp, body = self.http_client.get(url) + header = header or {} + resp, body = self.http_client.get(url, headers=header) body.pop('self', None) body['name'] = prop_name - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_generator() def list(self, namespace, **kwargs): """Retrieve a listing of metadata properties. @@ -295,18 +326,22 @@ class PropertyController(object): for key, value in body['properties'].items(): value['name'] = key - yield self.model(value) + yield self.model(value), resp + @utils.add_req_id_to_object() def delete(self, namespace, prop_name): """Delete a property.""" url = '/v2/metadefs/namespaces/{0}/properties/{1}'.format(namespace, prop_name) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp + @utils.add_req_id_to_object() def delete_all(self, namespace): """Delete all properties in a namespace.""" url = '/v2/metadefs/namespaces/{0}/properties'.format(namespace) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp class ObjectController(object): @@ -320,6 +355,7 @@ class ObjectController(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def create(self, namespace, **kwargs): """Create an object. @@ -335,7 +371,7 @@ class ObjectController(object): resp, body = self.http_client.post(url, data=obj) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp def update(self, namespace, object_name, **kwargs): """Update an object. @@ -359,17 +395,28 @@ class ObjectController(object): url = '/v2/metadefs/namespaces/{0}/objects/{1}'.format(namespace, object_name) - self.http_client.put(url, data=obj) + # Pass the original wrapped value to http client. + resp, _ = self.http_client.put(url, data=obj.wrapped) + # Get request id from `put` request so it can be passed to the + # following `get` call + req_id_hdr = { + 'x-openstack-request-id': utils._extract_request_id(resp)} - return self.get(namespace, obj.name) + return self._get(namespace, obj.name, req_id_hdr) def get(self, namespace, object_name): + return self._get(namespace, object_name) + + @utils.add_req_id_to_object() + def _get(self, namespace, object_name, header=None): url = '/v2/metadefs/namespaces/{0}/objects/{1}'.format(namespace, object_name) - resp, body = self.http_client.get(url) + header = header or {} + resp, body = self.http_client.get(url, headers=header) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_generator() def list(self, namespace, **kwargs): """Retrieve a listing of metadata objects. @@ -379,18 +426,22 @@ class ObjectController(object): resp, body = self.http_client.get(url) for obj in body['objects']: - yield self.model(obj) + yield self.model(obj), resp + @utils.add_req_id_to_object() def delete(self, namespace, object_name): """Delete an object.""" url = '/v2/metadefs/namespaces/{0}/objects/{1}'.format(namespace, object_name) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp + @utils.add_req_id_to_object() def delete_all(self, namespace): """Delete all objects in a namespace.""" url = '/v2/metadefs/namespaces/{0}/objects'.format(namespace) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp class TagController(object): @@ -404,6 +455,7 @@ class TagController(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_object() def create(self, namespace, tag_name): """Create a tag. @@ -416,8 +468,9 @@ class TagController(object): resp, body = self.http_client.post(url) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_generator() def create_multiple(self, namespace, **kwargs): """Create the list of tags. @@ -440,7 +493,7 @@ class TagController(object): resp, body = self.http_client.post(url, data=tags) body.pop('self', None) for tag in body['tags']: - yield self.model(tag) + yield self.model(tag), resp def update(self, namespace, tag_name, **kwargs): """Update a tag. @@ -464,17 +517,28 @@ class TagController(object): url = '/v2/metadefs/namespaces/{0}/tags/{1}'.format(namespace, tag_name) - self.http_client.put(url, data=tag) + # Pass the original wrapped value to http client. + resp, _ = self.http_client.put(url, data=tag.wrapped) + # Get request id from `put` request so it can be passed to the + # following `get` call + req_id_hdr = { + 'x-openstack-request-id': utils._extract_request_id(resp)} - return self.get(namespace, tag.name) + return self._get(namespace, tag.name, req_id_hdr) def get(self, namespace, tag_name): + return self._get(namespace, tag_name) + + @utils.add_req_id_to_object() + def _get(self, namespace, tag_name, header=None): url = '/v2/metadefs/namespaces/{0}/tags/{1}'.format(namespace, tag_name) - resp, body = self.http_client.get(url) + header = header or {} + resp, body = self.http_client.get(url, headers=header) body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_generator() def list(self, namespace, **kwargs): """Retrieve a listing of metadata tags. @@ -484,15 +548,19 @@ class TagController(object): resp, body = self.http_client.get(url) for tag in body['tags']: - yield self.model(tag) + yield self.model(tag), resp + @utils.add_req_id_to_object() def delete(self, namespace, tag_name): """Delete a tag.""" url = '/v2/metadefs/namespaces/{0}/tags/{1}'.format(namespace, tag_name) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp + @utils.add_req_id_to_object() def delete_all(self, namespace): """Delete all tags in a namespace.""" url = '/v2/metadefs/namespaces/{0}/tags'.format(namespace) - self.http_client.delete(url) + resp, body = self.http_client.delete(url) + return (resp, body), resp diff --git a/glanceclient/v2/tasks.py b/glanceclient/v2/tasks.py index 9c78020b..177c8bf7 100644 --- a/glanceclient/v2/tasks.py +++ b/glanceclient/v2/tasks.py @@ -38,6 +38,7 @@ class Controller(object): return warlock.model_factory(schema.raw(), base_class=schemas.SchemaBasedModel) + @utils.add_req_id_to_generator() def list(self, **kwargs): """Retrieve a listing of Task objects. @@ -48,14 +49,14 @@ class Controller(object): def paginate(url): resp, body = self.http_client.get(url) for task in body['tasks']: - yield task + yield task, resp try: next_url = body['next'] except KeyError: return else: - for task in paginate(next_url): - yield task + for task, resp in paginate(next_url): + yield task, resp filters = kwargs.get('filters', {}) @@ -88,12 +89,13 @@ class Controller(object): filters[param] = encodeutils.safe_encode(value) url = '/v2/tasks?%s' % six.moves.urllib.parse.urlencode(filters) - for task in paginate(url): + for task, resp in paginate(url): # NOTE(flwang): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict task.pop('self', None) - yield self.model(**task) + yield self.model(**task), resp + @utils.add_req_id_to_object() def get(self, task_id): """Get a task based on given task id.""" url = '/v2/tasks/%s' % task_id @@ -101,8 +103,9 @@ class Controller(object): # NOTE(flwang): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict body.pop('self', None) - return self.model(**body) + return self.model(**body), resp + @utils.add_req_id_to_object() def create(self, **kwargs): """Create a new task.""" url = '/v2/tasks' @@ -118,4 +121,4 @@ class Controller(object): # NOTE(flwang): remove 'self' for now until we have an elegant # way to pass it into the model constructor without conflict body.pop('self', None) - return self.model(**body) + return self.model(**body), resp diff --git a/releasenotes/notes/return-request-id-to-caller-47f4c0a684b1d88e.yaml b/releasenotes/notes/return-request-id-to-caller-47f4c0a684b1d88e.yaml new file mode 100644 index 00000000..25fe2e6e --- /dev/null +++ b/releasenotes/notes/return-request-id-to-caller-47f4c0a684b1d88e.yaml @@ -0,0 +1,10 @@ +--- +features: + - Added support to return "x-openstack-request-id" header in request_ids attribute + for better tracing. + + | For ex. + | >>> from glanceclient import Client + | >>> glance = Client('2', endpoint='OS_IMAGE_ENDPOINT', token='OS_AUTH_TOKEN') + | >>> res = glance.images.get('') + | >>> res.request_ids diff --git a/requirements.txt b/requirements.txt index eebfbfdc..cf010770 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,4 @@ warlock!=1.3.0,<2,>=1.0.1 # Apache-2.0 six>=1.9.0 # MIT oslo.utils>=3.18.0 # Apache-2.0 oslo.i18n>=2.1.0 # Apache-2.0 +wrapt>=1.7.0 # BSD License