diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9a3c2a11bc..e6cd6faf4d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -257,6 +257,22 @@ Improvements Contributed by @cognifloyd. +* Update majority of the "resource get" CLI commands (e.g. ``st2 execution get``, + ``st2 action get``, ``st2 rule get``, ``st2 pack get``, ``st2 apikey get``, ``st2 trace get``, + ``st2 key get``, ``st2 webhook get``, ``st2 timer get``, etc.) so they allow for retrieval + and printing of information for multiple resources using the following notation: + ``st2 get ``, e.g. ``st2 action.get pack.show packs.get + packs.delete`` + + This change is fully backward compatible when retrieving only a single resource (aka single + id is passed to the command). + + When retrieving a single source the command will throw and exit with non-zero if a resource is + not found, but when retrieving multiple resources, command will just print an error and + continue with printing the details of any other found resources. (new feature) #4912 + + Contributed by @Kami. + Fixed ~~~~~ diff --git a/st2actions/st2actions/notifier/config.py b/st2actions/st2actions/notifier/config.py index e5cb03366e..d5bb4b44ab 100644 --- a/st2actions/st2actions/notifier/config.py +++ b/st2actions/st2actions/notifier/config.py @@ -21,8 +21,6 @@ from st2common.constants.system import VERSION_STRING from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH -common_config.register_opts() - CONF = cfg.CONF @@ -44,7 +42,7 @@ def get_logging_config_path(): def _register_common_opts(ignore_errors=False): - common_config.register_opts() + common_config.register_opts(ignore_errors=ignore_errors) def _register_notifier_opts(ignore_errors=False): diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 1b97e8981a..17a0627871 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -1473,6 +1473,7 @@ class ActionExecutionGetCommand(ActionRunCommandMixin, ResourceViewCommand): "end_timestamp", "log", ] + pk_argument_name = "id" def __init__(self, resource, *args, **kwargs): super(ActionExecutionGetCommand, self).__init__( @@ -1484,7 +1485,9 @@ def __init__(self, resource, *args, **kwargs): ) self.parser.add_argument( - "id", help=("ID of the %s." % resource.get_display_name().lower()) + "id", + nargs="+", + help=("ID of the %s." % resource.get_display_name().lower()), ) self.parser.add_argument( "-x", @@ -1510,21 +1513,21 @@ def run(self, args, **kwargs): if args.exclude_result: kwargs["params"] = {"exclude_attributes": "result"} - execution = self.get_resource_by_id(id=args.id, **kwargs) - return execution + resource_ids = getattr(args, self.pk_argument_name, None) + resources = self._get_multiple_resources( + resource_ids=resource_ids, kwargs=kwargs + ) + return resources @add_auth_token_to_kwargs_from_cli def run_and_print(self, args, **kwargs): - try: - execution = self.run(args, **kwargs) + executions = self.run(args, **kwargs) + for execution in executions: if not args.json and not args.yaml: # Include elapsed time for running executions execution = format_execution_status(execution) - except resource.ResourceNotFoundError: - self.print_not_found(args.id) - raise ResourceNotFoundError("Execution with id %s not found." % (args.id)) - return self._print_execution_details(execution=execution, args=args, **kwargs) + self._print_execution_details(execution=execution, args=args, **kwargs) class ActionExecutionCancelCommand(resource.ResourceCommand): diff --git a/st2client/st2client/commands/inquiry.py b/st2client/st2client/commands/inquiry.py index d9395a5c54..8b265bcceb 100644 --- a/st2client/st2client/commands/inquiry.py +++ b/st2client/st2client/commands/inquiry.py @@ -127,11 +127,6 @@ class InquiryGetCommand(resource.ResourceGetCommand): def __init__(self, kv_resource, *args, **kwargs): super(InquiryGetCommand, self).__init__(kv_resource, *args, **kwargs) - @resource.add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_name = getattr(args, self.pk_argument_name, None) - return self.get_resource_by_id(id=resource_name, **kwargs) - class InquiryRespondCommand(resource.ResourceCommand): display_attributes = ["id", "response"] diff --git a/st2client/st2client/commands/keyvalue.py b/st2client/st2client/commands/keyvalue.py index e87f6afa35..19ea0d9f5b 100644 --- a/st2client/st2client/commands/keyvalue.py +++ b/st2client/st2client/commands/keyvalue.py @@ -208,12 +208,16 @@ def __init__(self, kv_resource, *args, **kwargs): @resource.add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): - resource_name = getattr(args, self.pk_argument_name, None) decrypt = getattr(args, "decrypt", False) scope = getattr(args, "scope", DEFAULT_GET_SCOPE) kwargs["params"] = {"decrypt": str(decrypt).lower()} kwargs["params"]["scope"] = scope - return self.get_resource_by_id(id=resource_name, **kwargs) + + resource_ids = getattr(args, self.pk_argument_name, None) + resources = self._get_multiple_resources( + resource_ids=resource_ids, kwargs=kwargs + ) + return resources class KeyValuePairSetCommand(resource.ResourceCommand): diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index da0fbc85e3..d66e990a93 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -15,6 +15,10 @@ from __future__ import absolute_import +from typing import List +from typing import Any +from typing import Dict + import os import abc import six @@ -238,6 +242,35 @@ def get_resource_by_ref_or_id(self, ref_or_id, **kwargs): raise ResourceNotFoundError(message) return instance + def _get_multiple_resources( + self, resource_ids: List[str], kwargs: Dict[str, Any] + ) -> List[Any]: + """ + Return multiple resource instances for the provided resource ids. + If a resource is not found, an error is printed. This method only throws when operating on + a single resource. + :param resource_ids: A list of resources to retrieve instances for. + :param kwargs: Dictionary with keyword arguments which are passed to get_resource_by_id. + """ + more_than_one_resource = len(resource_ids) > 1 + + resources = [] + for resource_id in resource_ids: + try: + resource = self.get_resource_by_id(resource_id, **kwargs) + except ResourceNotFoundError: + self.print_not_found(resource_id) + + if not more_than_one_resource: + # For backward compatibility reasons and to comply with common "get one" + # behavior, we only fail if a single source is requested + raise ResourceNotFoundError("Resource %s not found." % resource_id) + + continue + + resources.append(resource) + return resources + @abc.abstractmethod def run(self, args, **kwargs): raise NotImplementedError @@ -421,7 +454,7 @@ def __init__(self, resource, *args, **kwargs): resource=resource, argument=self.pk_argument_name ) - self.parser.add_argument(argument, metavar=metavar, help=help) + self.parser.add_argument(argument, metavar=metavar, nargs="+", help=help) self.parser.add_argument( "-a", "--attr", @@ -436,12 +469,16 @@ def __init__(self, resource, *args, **kwargs): @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) - return self.get_resource_by_id(resource_id, **kwargs) + resource_ids = getattr(args, self.pk_argument_name, None) + resources = self._get_multiple_resources( + resource_ids=resource_ids, kwargs=kwargs + ) + return resources def run_and_print(self, args, **kwargs): - try: - instance = self.run(args, **kwargs) + instances = self.run(args, **kwargs) + + for instance in instances: self.print_output( instance, table.PropertyValueTable, @@ -450,10 +487,6 @@ def run_and_print(self, args, **kwargs): yaml=args.yaml, attribute_display_order=self.attribute_display_order, ) - except ResourceNotFoundError: - resource_id = getattr(args, self.pk_argument_name, None) - self.print_not_found(resource_id) - raise OperationFailureException("Resource %s not found." % resource_id) class ContentPackResourceGetCommand(ResourceGetCommand): diff --git a/st2client/st2client/commands/rule_enforcement.py b/st2client/st2client/commands/rule_enforcement.py index dd624d4a72..fae09ef606 100644 --- a/st2client/st2client/commands/rule_enforcement.py +++ b/st2client/st2client/commands/rule_enforcement.py @@ -56,11 +56,6 @@ class RuleEnforcementGetCommand(resource.ResourceGetCommand): pk_argument_name = "id" - @resource.add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) - return self.get_resource_by_id(resource_id, **kwargs) - class RuleEnforcementListCommand(resource.ResourceCommand): display_attributes = [ diff --git a/st2client/st2client/commands/trace.py b/st2client/st2client/commands/trace.py index ac8de676c2..6c311221f1 100644 --- a/st2client/st2client/commands/trace.py +++ b/st2client/st2client/commands/trace.py @@ -16,7 +16,6 @@ from __future__ import absolute_import from st2client.models import Resource, Trace, TriggerInstance, Rule, Execution -from st2client.exceptions.operations import OperationFailureException from st2client.formatters import table from st2client.formatters import execution as execution_formatter from st2client.commands import resource @@ -327,22 +326,22 @@ def __init__(self, resource, *args, **kwargs): @resource.add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) - return self.get_resource_by_id(resource_id, **kwargs) + resource_ids = getattr(args, self.pk_argument_name, None) + resources = self._get_multiple_resources( + resource_ids=resource_ids, kwargs=kwargs + ) + return resources @resource.add_auth_token_to_kwargs_from_cli def run_and_print(self, args, **kwargs): - trace = None - try: - trace = self.run(args, **kwargs) - except resource.ResourceNotFoundError: - self.print_not_found(args.id) - raise OperationFailureException("Trace %s not found." % (args.id)) - # First filter for causation chains - trace = self._filter_trace_components(trace=trace, args=args) - # next filter for display purposes - trace = self._apply_display_filters(trace=trace, args=args) - return self.print_trace_details(trace=trace, args=args) + traces = self.run(args, **kwargs) + + for trace in traces: + # First filter for causation chains + trace = self._filter_trace_components(trace=trace, args=args) + # next filter for display purposes + trace = self._apply_display_filters(trace=trace, args=args) + self.print_trace_details(trace=trace, args=args) @staticmethod def _filter_trace_components(trace, args): diff --git a/st2client/st2client/commands/triggerinstance.py b/st2client/st2client/commands/triggerinstance.py index 12966fea92..3ccbaab2a5 100644 --- a/st2client/st2client/commands/triggerinstance.py +++ b/st2client/st2client/commands/triggerinstance.py @@ -212,8 +212,3 @@ class TriggerInstanceGetCommand(resource.ResourceGetCommand): attribute_display_order = ["id", "trigger", "occurrence_time", "payload"] pk_argument_name = "id" - - @resource.add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) - return self.get_resource_by_id(resource_id, **kwargs) diff --git a/st2client/st2client/models/reactor.py b/st2client/st2client/models/reactor.py index ef4c054f69..0d89d8cdd8 100644 --- a/st2client/st2client/models/reactor.py +++ b/st2client/st2client/models/reactor.py @@ -38,7 +38,7 @@ class TriggerType(core.Resource): class TriggerInstance(core.Resource): _alias = "Trigger-Instance" - _display_name = "TriggerInstance" + _display_name = "Trigger Instance" _plural = "Triggerinstances" _plural_display_name = "TriggerInstances" _repr_attributes = ["id", "trigger", "occurrence_time", "payload", "status"] diff --git a/st2client/tests/base.py b/st2client/tests/base.py index c06b2cadb4..e2ea29a3ae 100644 --- a/st2client/tests/base.py +++ b/st2client/tests/base.py @@ -125,7 +125,7 @@ def tearDown(self): print("") print("Captured stdout: %s" % (stdout)) - print("Captured stdoerr: %s" % (stderr)) + print("Captured stderr: %s" % (stderr)) print("") def _reset_output_streams(self): diff --git a/st2client/tests/unit/test_commands.py b/st2client/tests/unit/test_commands.py index 6dc5803b2c..729b671cd4 100644 --- a/st2client/tests/unit/test_commands.py +++ b/st2client/tests/unit/test_commands.py @@ -38,6 +38,70 @@ LOG = logging.getLogger(__name__) +class TestCommands(base.BaseCLITestCase): + def __init__(self, *args, **kwargs): + super(TestCommands, self).__init__(*args, **kwargs) + self.shell = Shell() + + @mock.patch.object( + httpclient.HTTPClient, + "get", + mock.MagicMock( + return_value=base.FakeResponse(json.dumps({}), 404, "NOT FOUND") + ), + ) + def test_all_resources_get_multi(self): + # 1. Verify that st2 get ... notation works for all get commands + resources = [ + ("action", models.Action), + ("action-alias", models.ActionAlias), + ("rule", models.Rule), + ("sensor", models.Sensor), + ("pack", models.Pack), + ("execution", models.Execution), + ("key", models.KeyValuePair), + ("webhook", models.Webhook), + ("trigger-instance", models.TriggerInstance), + ("trigger", models.TriggerType), + ("apikey", models.ApiKey), + ("inquiry", models.Inquiry), + ("policy", models.Policy), + ("policy-type", models.PolicyType), + ("timer", models.Timer), + ("trace", models.Trace), + ("runner", models.RunnerType), + ("rule-enforcement", models.RuleEnforcement), + ("role", models.Role), + ("role-assignment", models.UserRoleAssignment), + ] + + for command_name, resource_ in resources: + display_name = resource_.get_display_name() + + self._reset_output_streams() + return_code = self.shell.run([command_name, "get", "id1", "id2", "id3"]) + self.assertEqual(return_code, 0) + + stdout = self.stdout.getvalue() + self.assertTrue('%s "id1" is not found.' % (display_name) in stdout) + self.assertTrue('%s "id2" is not found.' % (display_name) in stdout) + self.assertTrue('%s "id3" is not found.' % (display_name) in stdout) + self._reset_output_streams() + + # 2. When a single id is provided, command should return non-zero in case resource is not + # found + for command_name, resource_ in resources: + display_name = resource_.get_display_name() + + self._reset_output_streams() + return_code = self.shell.run([command_name, "get", "id3"]) + self.assertEqual(return_code, 1) + + stdout = self.stdout.getvalue() + self.assertTrue('%s "id3" is not found.' % (display_name) in stdout) + self._reset_output_streams() + + class TestResourceCommand(unittest2.TestCase): def __init__(self, *args, **kwargs): super(TestResourceCommand, self).__init__(*args, **kwargs) @@ -84,8 +148,8 @@ def test_command_list_failed(self): def test_command_get_by_id(self): args = self.parser.parse_args(["fakeresource", "get", "123"]) self.assertEqual(args.func, self.branch.commands["get"].run_and_print) - instance = self.branch.commands["get"].run(args) - actual = instance.serialize() + instances = self.branch.commands["get"].run(args) + actual = instances[0].serialize() expected = json.loads(json.dumps(base.RESOURCES[0])) self.assertEqual(actual, expected) @@ -96,14 +160,40 @@ def test_command_get_by_id(self): return_value=base.FakeResponse(json.dumps(base.RESOURCES[0]), 200, "OK") ), ) - def test_command_get(self): + def test_command_get_single(self): args = self.parser.parse_args(["fakeresource", "get", "abc"]) self.assertEqual(args.func, self.branch.commands["get"].run_and_print) - instance = self.branch.commands["get"].run(args) + instances = self.branch.commands["get"].run(args) + self.assertEqual(len(instances), 1) + instance = instances[0] actual = instance.serialize() expected = json.loads(json.dumps(base.RESOURCES[0])) self.assertEqual(actual, expected) + @mock.patch.object( + httpclient.HTTPClient, + "get", + mock.MagicMock( + side_effect=[ + base.FakeResponse(json.dumps(base.RESOURCES[0]), 200, "OK"), + base.FakeResponse(json.dumps(base.RESOURCES[1]), 200, "OK"), + ] + ), + ) + def test_command_get_multiple(self): + args = self.parser.parse_args(["fakeresource", "get", "abc", "def"]) + self.assertEqual(args.func, self.branch.commands["get"].run_and_print) + instances = self.branch.commands["get"].run(args) + self.assertEqual(len(instances), 2) + + actual = instances[0].serialize() + expected = json.loads(json.dumps(base.RESOURCES[0])) + self.assertEqual(actual, expected) + + actual = instances[1].serialize() + expected = json.loads(json.dumps(base.RESOURCES[1])) + self.assertEqual(actual, expected) + @mock.patch.object( httpclient.HTTPClient, "get", @@ -305,8 +395,8 @@ def test_command_get_unicode_primary_key(self): ["fakeresource", "get", "examples.test_rule_utf8_náme"] ) self.assertEqual(args.func, self.branch.commands["get"].run_and_print) - instance = self.branch.commands["get"].run(args) - actual = instance.serialize() + instances = self.branch.commands["get"].run(args) + actual = instances[0].serialize() expected = json.loads(json.dumps(base.RESOURCES[0])) self.assertEqual(actual, expected) diff --git a/st2client/tests/unit/test_inquiry.py b/st2client/tests/unit/test_inquiry.py index 4132fda0d1..5ae225c79e 100644 --- a/st2client/tests/unit/test_inquiry.py +++ b/st2client/tests/unit/test_inquiry.py @@ -151,9 +151,11 @@ def test_get_inquiry_not_found(self): args = ["inquiry", "get", inquiry_id] retcode = self.shell.run(args) self.assertEqual( - 'Inquiry "%s" is not found.\n\n' % inquiry_id, self.stdout.getvalue() + 'Inquiry "%s" is not found.\n\nERROR: Resource asdbv not found.\n\n' + % inquiry_id, + self.stdout.getvalue(), ) - self.assertEqual(retcode, 2) + self.assertEqual(retcode, 1) @mock.patch.object( requests, diff --git a/st2client/tests/unit/test_shell.py b/st2client/tests/unit/test_shell.py index 32f128a506..aa54839ff6 100644 --- a/st2client/tests/unit/test_shell.py +++ b/st2client/tests/unit/test_shell.py @@ -841,7 +841,7 @@ def test_get_one_unicode_character_in_name(self): argv = ["action", "get", "examples.test_rule_utf8_náme"] args = shell.parser.parse_args(args=argv) shell.get_client(args=args) - self.assertEqual(args.ref_or_id, "examples.test_rule_utf8_náme") + self.assertEqual(args.ref_or_id, ["examples.test_rule_utf8_náme"]) def test_reencode_list_replace_surrogate_escape(self): value = ["a", "b", "c", "d"] diff --git a/st2exporter/st2exporter/config.py b/st2exporter/st2exporter/config.py index f5f6444891..4b43550ef2 100644 --- a/st2exporter/st2exporter/config.py +++ b/st2exporter/st2exporter/config.py @@ -25,8 +25,6 @@ from st2common.constants.system import VERSION_STRING from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH -common_config.register_opts() - CONF = cfg.CONF diff --git a/st2reactor/st2reactor/garbage_collector/config.py b/st2reactor/st2reactor/garbage_collector/config.py index af579245f0..4c6bb67255 100644 --- a/st2reactor/st2reactor/garbage_collector/config.py +++ b/st2reactor/st2reactor/garbage_collector/config.py @@ -23,8 +23,6 @@ from st2common.constants.garbage_collection import DEFAULT_COLLECTION_INTERVAL from st2common.constants.garbage_collection import DEFAULT_SLEEP_DELAY -common_config.register_opts() - CONF = cfg.CONF diff --git a/st2reactor/st2reactor/rules/config.py b/st2reactor/st2reactor/rules/config.py index 49922b2314..20bc720ec9 100644 --- a/st2reactor/st2reactor/rules/config.py +++ b/st2reactor/st2reactor/rules/config.py @@ -21,8 +21,6 @@ from st2common.constants.system import VERSION_STRING from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH -common_config.register_opts() - CONF = cfg.CONF