From a83c1f0a4244d6c422a68cf5bf2bbaa3e4617b78 Mon Sep 17 00:00:00 2001 From: Tang Chen Date: Tue, 15 Dec 2015 11:40:23 +0800 Subject: [PATCH] Network: Abstract get_body() out to be a private helper. get_body() is needed in each network files to construct a dict to pass to sdk proxy. And it is also used by several functions in each file. So define it as a file level private helper function. The unified prototype should be: def _get_attrs(client_manager, parsed_args): 1. The name, in sdk, the parameter passed to proxy is named "attrs". And it is a private method. So let's call it _get_attrs(). 2. The parameters, besides parsed_args, when we deal with project and project_domain, we have to make use of identity_client. So let's pass in the client manager. Change-Id: Ib044ebd4ddedbcd805f46334a7fe99e4ebb5b249 --- openstackclient/network/v2/network.py | 65 ++++++++++--------- openstackclient/tests/network/v2/fakes.py | 1 - .../tests/network/v2/test_network.py | 19 +++--- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/openstackclient/network/v2/network.py b/openstackclient/network/v2/network.py index fc94fd8277..6123721963 100644 --- a/openstackclient/network/v2/network.py +++ b/openstackclient/network/v2/network.py @@ -47,6 +47,33 @@ def _get_columns(item): return tuple(sorted(columns)) +def _get_attrs(client_manager, parsed_args): + attrs = {} + if parsed_args.name is not None: + attrs['name'] = str(parsed_args.name) + if parsed_args.admin_state is not None: + attrs['admin_state_up'] = parsed_args.admin_state + if parsed_args.shared is not None: + attrs['shared'] = parsed_args.shared + + # "network set" command doesn't support setting project. + if 'project' in parsed_args and parsed_args.project is not None: + identity_client = client_manager.identity + project_id = identity_common.find_project( + identity_client, + parsed_args.project, + parsed_args.project_domain, + ).id + attrs['tenant_id'] = project_id + + # "network set" command doesn't support setting availability zone hints. + if 'availability_zone_hints' in parsed_args and \ + parsed_args.availability_zone_hints is not None: + attrs['availability_zone_hints'] = parsed_args.availability_zone_hints + + return attrs + + class CreateNetwork(command.ShowOne): """Create new network""" @@ -105,31 +132,14 @@ class CreateNetwork(command.ShowOne): def take_action(self, parsed_args): client = self.app.client_manager.network - body = self.get_body(parsed_args) - obj = client.create_network(**body) + + attrs = _get_attrs(self.app.client_manager, parsed_args) + obj = client.create_network(**attrs) columns = _get_columns(obj) + data = utils.get_item_properties(obj, columns, formatters=_formatters) return (columns, data) - def get_body(self, parsed_args): - body = {'name': str(parsed_args.name), - 'admin_state_up': parsed_args.admin_state} - if parsed_args.shared is not None: - body['shared'] = parsed_args.shared - if parsed_args.project is not None: - identity_client = self.app.client_manager.identity - project_id = identity_common.find_project( - identity_client, - parsed_args.project, - parsed_args.project_domain, - ).id - body['tenant_id'] = project_id - if parsed_args.availability_zone_hints is not None: - body['availability_zone_hints'] = \ - parsed_args.availability_zone_hints - - return body - class DeleteNetwork(command.Command): """Delete network(s)""" @@ -271,18 +281,13 @@ class SetNetwork(command.Command): client = self.app.client_manager.network obj = client.find_network(parsed_args.identifier, ignore_missing=False) - if parsed_args.name is not None: - obj.name = str(parsed_args.name) - if parsed_args.admin_state is not None: - obj.admin_state_up = parsed_args.admin_state - if parsed_args.shared is not None: - obj.shared = parsed_args.shared - - if not obj.is_dirty: + attrs = _get_attrs(self.app.client_manager, parsed_args) + if attrs == {}: msg = "Nothing specified to be set" raise exceptions.CommandError(msg) - client.update_network(obj) + client.update_network(obj, **attrs) + return class ShowNetwork(command.ShowOne): diff --git a/openstackclient/tests/network/v2/fakes.py b/openstackclient/tests/network/v2/fakes.py index 4c862bd332..9d1dc5440b 100644 --- a/openstackclient/tests/network/v2/fakes.py +++ b/openstackclient/tests/network/v2/fakes.py @@ -96,7 +96,6 @@ class FakeNetwork(object): 'subnets': ['a', 'b'], 'provider_network_type': 'vlan', 'router_external': True, - 'is_dirty': True, 'availability_zones': [], 'availability_zone_hints': [], } diff --git a/openstackclient/tests/network/v2/test_network.py b/openstackclient/tests/network/v2/test_network.py index 37cc66742a..f96497a437 100644 --- a/openstackclient/tests/network/v2/test_network.py +++ b/openstackclient/tests/network/v2/test_network.py @@ -440,8 +440,6 @@ class TestSetNetwork(TestNetwork): self.cmd = network.SetNetwork(self.app, self.namespace) def test_set_this(self): - self._network.is_dirty = True - arglist = [ self._network.name, '--enable', @@ -458,12 +456,15 @@ class TestSetNetwork(TestNetwork): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.network.update_network.assert_called_with(self._network) + attrs = { + 'name': 'noob', + 'admin_state_up': True, + 'shared': True, + } + self.network.update_network.assert_called_with(self._network, **attrs) self.assertIsNone(result) def test_set_that(self): - self._network.is_dirty = True - arglist = [ self._network.name, '--disable', @@ -478,12 +479,14 @@ class TestSetNetwork(TestNetwork): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.network.update_network.assert_called_with(self._network) + attrs = { + 'admin_state_up': False, + 'shared': False, + } + self.network.update_network.assert_called_with(self._network, **attrs) self.assertIsNone(result) def test_set_nothing(self): - self._network.is_dirty = False - arglist = [self._network.name, ] verifylist = [('identifier', self._network.name), ]