From 22621e7b489f6d8ba9bb2ea229b29cf9f80cc359 Mon Sep 17 00:00:00 2001 From: Alexander Kislitsky Date: Mon, 19 Dec 2016 13:05:55 +0300 Subject: [PATCH] Sorting of output fixed for list commands Used sorting implementation from the BaseListCommand. Closes-Bug: #1614032 Change-Id: Iced41a12ec20712b5cb7fe4c2a21b3763e1713aa (cherry picked from commit f83b052de2ab471dae93e14a53fa9cfbd796ca21) --- fuelclient/commands/base.py | 14 +++++----- fuelclient/commands/node.py | 19 ++++++++------ fuelclient/commands/openstack_config.py | 1 + fuelclient/commands/plugins.py | 7 ----- fuelclient/commands/release.py | 26 ++++++++++++++----- fuelclient/commands/role.py | 5 ++++ fuelclient/commands/tag.py | 5 ++++ fuelclient/tests/unit/v2/cli/test_env.py | 6 +++++ fuelclient/tests/unit/v2/cli/test_node.py | 21 +++++++++++---- .../unit/v2/cli/test_openstack_config.py | 7 +++++ fuelclient/tests/unit/v2/cli/test_plugins.py | 6 +++++ fuelclient/tests/unit/v2/cli/test_release.py | 13 ++++++++++ fuelclient/tests/unit/v2/cli/test_role.py | 17 ++++++++++++ fuelclient/tests/unit/v2/cli/test_tag.py | 21 +++++++++++++++ fuelclient/v1/node.py | 2 -- 15 files changed, 135 insertions(+), 35 deletions(-) diff --git a/fuelclient/commands/base.py b/fuelclient/commands/base.py index 392d19c..6cb47b4 100644 --- a/fuelclient/commands/base.py +++ b/fuelclient/commands/base.py @@ -112,6 +112,12 @@ class BaseListCommand(lister.Lister, BaseCommand): return parser + def _sort_data(self, parsed_args, data): + scolumn_ids = [self.columns.index(col) + for col in parsed_args.sort_columns] + data.sort(key=lambda x: [x[scolumn_id] for scolumn_id in scolumn_ids]) + return data + def take_action(self, parsed_args): filters = {} for name, prop in self.filters.items(): @@ -121,13 +127,9 @@ class BaseListCommand(lister.Lister, BaseCommand): data = self.client.get_all(**filters) data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(parsed_args, data) - scolumn_ids = [self.columns.index(col) - for col in parsed_args.sort_columns] - - data.sort(key=lambda x: [x[scolumn_id] for scolumn_id in scolumn_ids]) - - return (self.columns, data) + return self.columns, data @six.add_metaclass(abc.ABCMeta) diff --git a/fuelclient/commands/node.py b/fuelclient/commands/node.py index 62313aa..120f0ec 100644 --- a/fuelclient/commands/node.py +++ b/fuelclient/commands/node.py @@ -179,6 +179,11 @@ class NodeList(NodeMixIn, base.BaseListCommand): 'platform_name', 'online') + filters = { + 'environment_id': 'env', + 'labels': 'labels' + } + def get_parser(self, prog_name): parser = super(NodeList, self).get_parser(prog_name) @@ -197,13 +202,6 @@ class NodeList(NodeMixIn, base.BaseListCommand): return parser - def take_action(self, parsed_args): - data = self.client.get_all( - environment_id=parsed_args.env, labels=parsed_args.labels) - data = data_utils.get_display_data_multi(self.columns, data) - - return (self.columns, data) - class NodeShow(NodeMixIn, base.BaseShowCommand): """Show info about node with given id.""" @@ -333,6 +331,10 @@ class NodeLabelList(NodeMixIn, base.BaseListCommand): 'label_name', 'label_value') + @property + def default_sorting_by(self): + return ['node_id'] + def get_parser(self, prog_name): parser = super(NodeLabelList, self).get_parser(prog_name) @@ -348,8 +350,9 @@ class NodeLabelList(NodeMixIn, base.BaseListCommand): data = self.client.get_all_labels_for_nodes( node_ids=parsed_args.nodes) data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(parsed_args, data) - return (self.columns, data) + return self.columns, data class NodeLabelSet(NodeMixIn, base.BaseCommand): diff --git a/fuelclient/commands/openstack_config.py b/fuelclient/commands/openstack_config.py index f5bde9b..8e38cf0 100644 --- a/fuelclient/commands/openstack_config.py +++ b/fuelclient/commands/openstack_config.py @@ -91,6 +91,7 @@ class OpenstackConfigList(OpenstackConfigMixin, base.BaseListCommand): cluster_id=args.env, node_ids=args.node, node_role=args.role, is_active=(not args.deleted)) data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(args, data) return self.columns, data diff --git a/fuelclient/commands/plugins.py b/fuelclient/commands/plugins.py index a0e3f0d..7949a23 100644 --- a/fuelclient/commands/plugins.py +++ b/fuelclient/commands/plugins.py @@ -13,7 +13,6 @@ # under the License. from fuelclient.commands import base -from fuelclient.common import data_utils class PluginsMixIn(object): @@ -29,12 +28,6 @@ class PluginsList(PluginsMixIn, base.BaseListCommand): 'package_version', 'releases') - def take_action(self, parsed_args): - data = self.client.get_all() - data = data_utils.get_display_data_multi(self.columns, data) - - return self.columns, data - class PluginsSync(PluginsMixIn, base.BaseCommand): """Synchronise plugins on file system with plugins in API service.""" diff --git a/fuelclient/commands/release.py b/fuelclient/commands/release.py index ea904d7..5edebcf 100644 --- a/fuelclient/commands/release.py +++ b/fuelclient/commands/release.py @@ -36,10 +36,17 @@ class ReleaseList(ReleaseMixIn, base.BaseListCommand): class ReleaseReposList(ReleaseMixIn, base.BaseListCommand): """Show repos for a given release.""" - def columns(self, repos): - if repos: - return tuple(repos[0]) - return () + columns = ( + 'name', + 'priority', + 'uri', + 'section', + 'suite', + 'type') + + @property + def default_sorting_by(self): + return ['priority'] def get_parser(self, prog_name): parser = super(ReleaseReposList, self).get_parser(prog_name) @@ -50,9 +57,9 @@ class ReleaseReposList(ReleaseMixIn, base.BaseListCommand): def take_action(self, parsed_args): data = self.client.get_attributes_metadata_by_id(parsed_args.id) repos = data["editable"]["repo_setup"]["repos"]["value"] - columns = self.columns(repos) - repos = data_utils.get_display_data_multi(columns, repos) - return columns, repos + repos = data_utils.get_display_data_multi(self.columns, repos) + repos = self._sort_data(parsed_args, repos) + return self.columns, repos class ReleaseReposUpdate(ReleaseMixIn, base.BaseCommand): @@ -90,6 +97,10 @@ class ReleaseComponentList(ReleaseMixIn, base.BaseListCommand): "incompatible", "default") + @property + def default_sorting_by(self): + return ['name'] + @staticmethod def retrieve_predicates(statement): """Retrieve predicates with respective 'items' components @@ -131,4 +142,5 @@ class ReleaseComponentList(ReleaseMixIn, base.BaseListCommand): data = [{k: self.retrieve_data(d.get(k, '-')) for k in self.columns} for d in data] data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(parsed_args, data) return self.columns, data diff --git a/fuelclient/commands/role.py b/fuelclient/commands/role.py index 555e7f3..82fe3bc 100644 --- a/fuelclient/commands/role.py +++ b/fuelclient/commands/role.py @@ -127,6 +127,10 @@ class RoleList(RoleMixIn, base.BaseListCommand): "conflicts", "description") + @property + def default_sorting_by(self): + return ['name'] + def get_parser(self, prog_name): parser = super(RoleList, self).get_parser(prog_name) group = parser.add_mutually_exclusive_group(required=True) @@ -146,6 +150,7 @@ class RoleList(RoleMixIn, base.BaseListCommand): data = self.client.get_all(model, model_id) data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(parsed_args, data) return self.columns, data diff --git a/fuelclient/commands/tag.py b/fuelclient/commands/tag.py index 5fc6d6b..8f22014 100644 --- a/fuelclient/commands/tag.py +++ b/fuelclient/commands/tag.py @@ -127,6 +127,10 @@ class TagList(TagMixIn, base.BaseListCommand): "conflicts", "description") + @property + def default_sorting_by(self): + return ['name'] + def get_parser(self, prog_name): parser = super(TagList, self).get_parser(prog_name) group = parser.add_mutually_exclusive_group(required=True) @@ -145,6 +149,7 @@ class TagList(TagMixIn, base.BaseListCommand): data = self.client.get_all(model, model_id) data = data_utils.get_display_data_multi(self.columns, data) + data = self._sort_data(parsed_args, data) return self.columns, data diff --git a/fuelclient/tests/unit/v2/cli/test_env.py b/fuelclient/tests/unit/v2/cli/test_env.py index b21285a..bada811 100644 --- a/fuelclient/tests/unit/v2/cli/test_env.py +++ b/fuelclient/tests/unit/v2/cli/test_env.py @@ -46,6 +46,12 @@ class TestEnvCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('environment', mock.ANY) self.m_client.get_all.assert_called_once_with() + def test_env_list_sorted(self): + args = 'env list -s name' + self.exec_command(args) + self.m_get_client.assert_called_once_with('environment', mock.ANY) + self.m_client.get_all.assert_called_once_with() + def test_env_show(self): args = 'env show 42' self.exec_command(args) diff --git a/fuelclient/tests/unit/v2/cli/test_node.py b/fuelclient/tests/unit/v2/cli/test_node.py index b0b9565..f933f89 100644 --- a/fuelclient/tests/unit/v2/cli/test_node.py +++ b/fuelclient/tests/unit/v2/cli/test_node.py @@ -43,8 +43,13 @@ class TestNodeCommand(test_engine.BaseCLITest): self.exec_command(args) self.m_get_client.assert_called_once_with('node', mock.ANY) - self.m_client.get_all.assert_called_once_with( - environment_id=None, labels=None) + self.m_client.get_all.assert_called_once_with() + + def test_node_list_sorted(self): + args = 'node list -s name' + self.exec_command(args) + self.m_get_client.assert_called_once_with('node', mock.ANY) + self.m_client.get_all.assert_called_once_with() def test_node_list_with_env(self): env_id = 42 @@ -54,7 +59,7 @@ class TestNodeCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('node', mock.ANY) self.m_client.get_all.assert_called_once_with( - environment_id=env_id, labels=None) + environment_id=env_id) def test_node_list_with_labels(self): labels = ['key_1=val_1', 'key_2=val_2', 'key3'] @@ -64,8 +69,7 @@ class TestNodeCommand(test_engine.BaseCLITest): self.exec_command(args) self.m_get_client.assert_called_once_with('node', mock.ANY) - self.m_client.get_all.assert_called_once_with( - environment_id=None, labels=labels) + self.m_client.get_all.assert_called_once_with(labels=labels) def test_node_list_with_env_and_labels(self): env_id = 42 @@ -240,6 +244,13 @@ node-4 ansible_host=10.20.0.5 self.m_client.get_all_labels_for_nodes.assert_called_once_with( node_ids=None) + def test_node_label_list_sorted(self): + args = 'node label list -s label_name' + self.exec_command(args) + self.m_get_client.assert_called_once_with('node', mock.ANY) + self.m_client.get_all_labels_for_nodes.assert_called_once_with( + node_ids=None) + def test_node_label_list_for_specific_nodes(self): node_ids = ['42', '43'] args = 'node label list --nodes {node_ids}'.format( diff --git a/fuelclient/tests/unit/v2/cli/test_openstack_config.py b/fuelclient/tests/unit/v2/cli/test_openstack_config.py index dade96a..4b91abc 100644 --- a/fuelclient/tests/unit/v2/cli/test_openstack_config.py +++ b/fuelclient/tests/unit/v2/cli/test_openstack_config.py @@ -54,6 +54,13 @@ class TestOpenstackConfig(test_engine.BaseCLITest): 'node_role': None, 'is_active': True} ) + def test_config_list_sorted(self): + self._test_config_list( + cmd_line='--env {0} -s node_id'.format(self.CLUSTER_ID), + expected_kwargs={'cluster_id': self.CLUSTER_ID, 'node_ids': None, + 'node_role': None, 'is_active': True} + ) + @mock.patch('sys.stderr') def test_config_list_for_cluster_fail(self, mocked_stderr): self.assertRaises(SystemExit, diff --git a/fuelclient/tests/unit/v2/cli/test_plugins.py b/fuelclient/tests/unit/v2/cli/test_plugins.py index 182eb67..629810a 100644 --- a/fuelclient/tests/unit/v2/cli/test_plugins.py +++ b/fuelclient/tests/unit/v2/cli/test_plugins.py @@ -37,6 +37,12 @@ class TestPluginsCommand(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('plugins', mock.ANY) self.m_client.get_all.assert_called_once_with() + def test_plugin_list_sorted(self): + args = 'plugins list -s name' + self.exec_command(args) + self.m_get_client.assert_called_once_with('plugins', mock.ANY) + self.m_client.get_all.assert_called_once_with() + def test_plugins_sync_all(self): args = 'plugins sync' self.exec_command(args) diff --git a/fuelclient/tests/unit/v2/cli/test_release.py b/fuelclient/tests/unit/v2/cli/test_release.py index 21d0cd9..6d397bc 100644 --- a/fuelclient/tests/unit/v2/cli/test_release.py +++ b/fuelclient/tests/unit/v2/cli/test_release.py @@ -43,6 +43,12 @@ class TestReleaseCommand(test_engine.BaseCLITest): self.m_client.get_attributes_metadata_by_id.assert_called_once_with(1) self.m_get_client.assert_called_once_with('release', mock.ANY) + def test_release_repos_list_sorted(self): + args = 'release repos list 1 -s name' + self.exec_command(args) + self.m_client.get_attributes_metadata_by_id.assert_called_once_with(1) + self.m_get_client.assert_called_once_with('release', mock.ANY) + @mock.patch('fuelclient.commands.release.utils.parse_yaml_file') def test_release_repos_update(self, mock_parse_yaml): args = 'release repos update 1 -f repos.yaml' @@ -72,3 +78,10 @@ class TestReleaseCommand(test_engine.BaseCLITest): self.exec_command(args) self.m_client.get_components_by_id.assert_called_once_with(release_id) self.m_get_client.assert_called_once_with('release', mock.ANY) + + def test_release_component_list_sorted(self): + release_id = 42 + args = 'release component list {0} -s default'.format(release_id) + self.exec_command(args) + self.m_client.get_components_by_id.assert_called_once_with(release_id) + self.m_get_client.assert_called_once_with('release', mock.ANY) diff --git a/fuelclient/tests/unit/v2/cli/test_role.py b/fuelclient/tests/unit/v2/cli/test_role.py index c2af5e1..d54f8c3 100644 --- a/fuelclient/tests/unit/v2/cli/test_role.py +++ b/fuelclient/tests/unit/v2/cli/test_role.py @@ -59,6 +59,23 @@ class TestRoleCommand(test_engine.BaseCLITest): self.m_client.get_all.assert_called_once_with('clusters', env_id) self.m_get_client.assert_called_once_with('role', mock.ANY) + def test_role_list_sorted(self): + self.m_client.get_all.return_value = [ + {"name": "fake_role_2", + "group": "fake_group_1", + "conflicts": ["fake_role_1", "fake_role_3"], + "description": "some fake description"}, + {"name": "fake_role_1", + "group": "fake_group_2", + "conflicts": ["fake_role_2", "fake_role_3"], + "description": "some fake description"}, + ] + env_id = 45 + args = 'role list -e {id} -s group'.format(id=env_id) + self.exec_command(args) + self.m_client.get_all.assert_called_once_with('clusters', env_id) + self.m_get_client.assert_called_once_with('role', mock.ANY) + @mock.patch('sys.stderr') def test_role_list_fail(self, mocked_stderr): args = 'role list' diff --git a/fuelclient/tests/unit/v2/cli/test_tag.py b/fuelclient/tests/unit/v2/cli/test_tag.py index 425b97e..c13fbef 100644 --- a/fuelclient/tests/unit/v2/cli/test_tag.py +++ b/fuelclient/tests/unit/v2/cli/test_tag.py @@ -63,6 +63,27 @@ class TestTagCommand(test_engine.BaseCLITest): self.m_client.get_all.assert_called_once_with('clusters', env_id) self.m_get_client.assert_called_once_with('tag', mock.ANY) + def test_tag_list_sorted(self): + self.m_client.get_all.return_value = [ + {"tag": "fake_tag_2", + "group": "group_1", + "has_primary": True, + "owner_id": 1, + "owner_type": 'release', + }, + {"tag": "fake_tag_1", + "group": "group_2", + "has_primary": True, + "owner_id": 1, + "owner_type": 'release', + } + ] + env_id = 45 + args = 'tag list -e {id} -s group'.format(id=env_id) + self.exec_command(args) + self.m_client.get_all.assert_called_once_with('clusters', env_id) + self.m_get_client.assert_called_once_with('tag', mock.ANY) + @mock.patch('sys.stderr') def test_tag_list_fail(self, mocked_stderr): args = 'tag list' diff --git a/fuelclient/v1/node.py b/fuelclient/v1/node.py index cc03fcd..187a69a 100644 --- a/fuelclient/v1/node.py +++ b/fuelclient/v1/node.py @@ -130,8 +130,6 @@ class NodeClient(base_v1.BaseV1Client): 'label_value': value }) - labels = sorted(labels, key=lambda label: label.get('node_id')) - return labels def set_labels_for_nodes(self, labels=None, node_ids=None):