From 75fcc4b4e910f2be929b3ae901c9fa88b481d96e Mon Sep 17 00:00:00 2001 From: Jaromir Wysoglad Date: Tue, 31 Oct 2023 05:31:58 -0400 Subject: [PATCH] Fix cli commands Something must have changed either with cliff or with a different version of python. All of the cli commands end with: "'Namespace' object is not subscriptable" This is caused because of how the parsed_args fields were accessed. This patch addresses the issue. Now it accesses the fields the same way as aodhclient does. The unit tests for cli now use the arg parser to actually test this. During this I also discovered incorrect mocking in the configuration tests. I added another mock to the test cases which were missing it. Change-Id: Ib5dbbdb48bdfd3214ec76acc4c68649e25f695db --- observabilityclient/tests/unit/test_cli.py | 60 ++++++++++++++------ observabilityclient/tests/unit/test_utils.py | 4 ++ observabilityclient/v1/cli.py | 16 +++--- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/observabilityclient/tests/unit/test_cli.py b/observabilityclient/tests/unit/test_cli.py index 0d56fd8..536de62 100644 --- a/observabilityclient/tests/unit/test_cli.py +++ b/observabilityclient/tests/unit/test_cli.py @@ -28,30 +28,31 @@ class CliTest(testtools.TestCase): self.client.query = mock.Mock() def test_list(self): - args_enabled = {'disable_rbac': False} - args_disabled = {'disable_rbac': True} - metric_names = ['name1', 'name2', 'name3'] expected = (['metric_name'], [['name1'], ['name2'], ['name3']]) cli_list = cli.List(mock.Mock(), mock.Mock()) + parser = cli_list.get_parser("metric list") + test_parsed_args_enabled = parser.parse_args([ + ]) + test_parsed_args_disabled = parser.parse_args([ + "--disable-rbac" + ]) + with mock.patch.object(metric_utils, 'get_client', return_value=self.client), \ mock.patch.object(self.client.query, 'list', return_value=metric_names) as m: - ret1 = cli_list.take_action(args_enabled) + ret1 = cli_list.take_action(test_parsed_args_enabled) m.assert_called_with(disable_rbac=False) - ret2 = cli_list.take_action(args_disabled) + ret2 = cli_list.take_action(test_parsed_args_disabled) m.assert_called_with(disable_rbac=True) self.assertEqual(ret1, expected) self.assertEqual(ret2, expected) def test_show(self): - args_enabled = {'name': 'metric_name', 'disable_rbac': False} - args_disabled = {'name': 'metric_name', 'disable_rbac': True} - metric = { 'value': [123456, 12], 'metric': {'label1': 'value1'} @@ -61,15 +62,24 @@ class CliTest(testtools.TestCase): cli_show = cli.Show(mock.Mock(), mock.Mock()) + parser = cli_show.get_parser("metric show") + test_parsed_args_enabled = parser.parse_args([ + "metric_name" + ]) + test_parsed_args_disabled = parser.parse_args([ + "metric_name", + "--disable-rbac" + ]) + with mock.patch.object(metric_utils, 'get_client', return_value=self.client), \ mock.patch.object(self.client.query, 'show', return_value=prom_metric) as m: - ret1 = cli_show.take_action(args_enabled) + ret1 = cli_show.take_action(test_parsed_args_enabled) m.assert_called_with('metric_name', disable_rbac=False) - ret2 = cli_show.take_action(args_disabled) + ret2 = cli_show.take_action(test_parsed_args_disabled) m.assert_called_with('metric_name', disable_rbac=True) self.assertEqual(ret1, expected) @@ -78,8 +88,6 @@ class CliTest(testtools.TestCase): def test_query(self): query = ("some_query{label!~'not_this_value'} - " "sum(second_metric{label='this'})") - args_enabled = {'query': query, 'disable_rbac': False} - args_disabled = {'query': query, 'disable_rbac': True} metric = { 'value': [123456, 12], @@ -91,32 +99,48 @@ class CliTest(testtools.TestCase): cli_query = cli.Query(mock.Mock(), mock.Mock()) + parser = cli_query.get_parser("metric query") + test_parsed_args_enabled = parser.parse_args([ + query + ]) + test_parsed_args_disabled = parser.parse_args([ + query, + "--disable-rbac" + ]) + with mock.patch.object(metric_utils, 'get_client', return_value=self.client), \ mock.patch.object(self.client.query, 'query', return_value=prom_metric) as m: - ret1 = cli_query.take_action(args_enabled) + ret1 = cli_query.take_action(test_parsed_args_enabled) m.assert_called_with(query, disable_rbac=False) - ret2 = cli_query.take_action(args_disabled) + ret2 = cli_query.take_action(test_parsed_args_disabled) m.assert_called_with(query, disable_rbac=True) self.assertEqual(ret1, expected) self.assertEqual(ret2, expected) def test_delete(self): - matches = "some_label_name" - args = {'matches': matches, 'start': 0, 'end': 10} + match1 = "some_label_name" + match2 = "some_label_name2" cli_delete = cli.Delete(mock.Mock(), mock.Mock()) + parser = cli_delete.get_parser("metric delete") + test_parsed_args = parser.parse_args([ + match1, match2, + "--start", "0", + "--end", "10" + ]) + with mock.patch.object(metric_utils, 'get_client', return_value=self.client), \ mock.patch.object(self.client.query, 'delete') as m: - cli_delete.take_action(args) - m.assert_called_with(matches, 0, 10) + cli_delete.take_action(test_parsed_args) + m.assert_called_with([[match1, match2]], "0", "10") def test_clean_combstones(self): cli_clean_tombstones = cli.CleanTombstones(mock.Mock(), mock.Mock()) diff --git a/observabilityclient/tests/unit/test_utils.py b/observabilityclient/tests/unit/test_utils.py index 871b856..2eabf43 100644 --- a/observabilityclient/tests/unit/test_utils.py +++ b/observabilityclient/tests/unit/test_utils.py @@ -70,6 +70,8 @@ class GetPrometheusClientTest(testtools.TestCase): patched_env = {'PROMETHEUS_HOST': 'env_overide', 'PROMETHEUS_PORT': 'env_port'} with mock.patch.dict(os.environ, patched_env), \ + mock.patch.object(metric_utils, 'get_config_file', + return_value=None), \ mock.patch.object(prometheus_client.PrometheusAPIClient, "__init__", return_value=None) as m: metric_utils.get_prometheus_client() @@ -77,6 +79,8 @@ class GetPrometheusClientTest(testtools.TestCase): def test_get_prometheus_client_missing_configuration(self): with mock.patch.dict(os.environ, {}), \ + mock.patch.object(metric_utils, 'get_config_file', + return_value=None), \ mock.patch.object(prometheus_client.PrometheusAPIClient, "__init__", return_value=None): self.assertRaises(metric_utils.ConfigurationError, diff --git a/observabilityclient/v1/cli.py b/observabilityclient/v1/cli.py index 7fd7509..d10fd7a 100644 --- a/observabilityclient/v1/cli.py +++ b/observabilityclient/v1/cli.py @@ -24,7 +24,7 @@ class List(base.ObservabilityBaseCommand, lister.Lister): def take_action(self, parsed_args): client = metric_utils.get_client(self) - metrics = client.query.list(disable_rbac=parsed_args['disable_rbac']) + metrics = client.query.list(disable_rbac=parsed_args.disable_rbac) return ["metric_name"], [[m] for m in metrics] @@ -40,8 +40,8 @@ class Show(base.ObservabilityBaseCommand, lister.Lister): def take_action(self, parsed_args): client = metric_utils.get_client(self) - metric = client.query.show(parsed_args['name'], - disable_rbac=parsed_args['disable_rbac']) + metric = client.query.show(parsed_args.name, + disable_rbac=parsed_args.disable_rbac) ret = metric_utils.metrics2cols(metric) return ret @@ -58,8 +58,8 @@ class Query(base.ObservabilityBaseCommand, lister.Lister): def take_action(self, parsed_args): client = metric_utils.get_client(self) - metric = client.query.query(parsed_args['query'], - disable_rbac=parsed_args['disable_rbac']) + metric = client.query.query(parsed_args.query, + disable_rbac=parsed_args.disable_rbac) ret = metric_utils.metrics2cols(metric) return ret @@ -88,9 +88,9 @@ class Delete(base.ObservabilityBaseCommand): def take_action(self, parsed_args): client = metric_utils.get_client(self) - return client.query.delete(parsed_args['matches'], - parsed_args['start'], - parsed_args['end']) + return client.query.delete(parsed_args.matches, + parsed_args.start, + parsed_args.end) class CleanTombstones(base.ObservabilityBaseCommand):