From 752a2db332780ac98c4a4c3a3778b178131a8d59 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 8 Jun 2018 11:46:29 -0400 Subject: [PATCH] Optimize _prep_server_detail to avoid redundant find_resource When showing a server or doing a rebuild, we already have the latest version of the server so _prep_server_detail getting the server again is an unnecessary performance hit. ShowServer is pretty obvious here. For RebuildServer, the compute API actually refreshes the server before returning it in the response, so the client already gets the latest when the rebuild call returns. The only other usage of _prep_server_detail that does require a refresh is CreateServer since the POST /servers response is a minimal version of the server object. This adds a new refresh kwarg, backward compatible by default, to _prep_server_detail but changes ShowServer and RebuildServer to no longer refresh. Change-Id: Ib1c9c424ed1cafc2dfd8be90af8de8a774bdfbf0 --- openstackclient/compute/v2/server.py | 17 +++++++++++------ .../tests/unit/compute/v2/test_server.py | 8 ++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 88016c0bc..c2d9cf3af 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -120,18 +120,21 @@ def _prefix_checked_value(prefix): return func -def _prep_server_detail(compute_client, image_client, server): +def _prep_server_detail(compute_client, image_client, server, refresh=True): """Prepare the detailed server dict for printing :param compute_client: a compute client instance :param image_client: an image client instance :param server: a Server resource + :param refresh: Flag indicating if ``server`` is already the latest version + or if it needs to be refreshed, for example when showing + the latest details of a server after creating it. :rtype: a dict of server details """ info = server.to_dict() - - server = utils.find_resource(compute_client.servers, info['id']) - info.update(server.to_dict()) + if refresh: + server = utils.find_resource(compute_client.servers, info['id']) + info.update(server.to_dict()) # Convert the image blob to a name image_info = info.get('image', {}) @@ -1540,7 +1543,8 @@ class RebuildServer(command.ShowOne): self.app.stdout.write(_('Error rebuilding server\n')) raise SystemExit - details = _prep_server_detail(compute_client, image_client, server) + details = _prep_server_detail(compute_client, image_client, server, + refresh=False) return zip(*sorted(six.iteritems(details))) @@ -2021,7 +2025,8 @@ class ShowServer(command.ShowOne): return ({}, {}) else: data = _prep_server_detail(compute_client, - self.app.client_manager.image, server) + self.app.client_manager.image, server, + refresh=False) return zip(*sorted(six.iteritems(data))) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index d242dc261..61c81132a 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -2331,17 +2331,17 @@ class TestServerRebuild(TestServer): self.images_mock.get.return_value = self.image # Fake the rebuilt new server. - new_server = compute_fakes.FakeServer.create_one_server() - - # Fake the server to be rebuilt. The IDs of them should be the same. attrs = { - 'id': new_server.id, 'image': { 'id': self.image.id }, 'networks': {}, 'adminPass': 'passw0rd', } + new_server = compute_fakes.FakeServer.create_one_server(attrs=attrs) + + # Fake the server to be rebuilt. The IDs of them should be the same. + attrs['id'] = new_server.id methods = { 'rebuild': new_server, }