From 8e4ce65f723556a3bec8c3799d39e0bce14e6398 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 15 2023 03:59:27 +0000 Subject: [PATCH 1/5] cli: streamline python/json options in call command --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index fc78c6d..59a2594 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -966,36 +966,70 @@ def handle_call(goptions, session, args): Note, that you can use global option --noauth for anonymous calls here""" usage = textwrap.dedent(usage) parser = OptionParser(usage=get_usage_str(usage)) - parser.add_option("--python", action="store_true", + parser.add_option("-p", "--python", action="store_true", help="Use python syntax for RPC parameter values") parser.add_option("--kwargs", - help="Specify keyword arguments as a dictionary (implies --python)") + help="Specify keyword arguments as a dictionary (implies --python or " + "--json-input)") + parser.add_option("-j", "--json", action="store_true", + help="Use JSON syntax for input and output") + parser.add_option("--json-input", action="store_true", help="Use JSON syntax for input") parser.add_option("--json-output", action="store_true", help="Use JSON syntax for output") + parser.add_option("-b", "--bare-strings", action="store_true", + help="Treat invalid json/python as bare strings") (options, args) = parser.parse_args(args) if len(args) < 1: parser.error("Please specify the name of the XML-RPC method") - if options.kwargs: + if options.json: + options.json_input = True + options.json_output = True + if options.python and options.json_input: + parser.error('The --python option conflicts with using --json-input') + if options.kwargs and not options.json_input: + # for backwards compatibility, --python is implied options.python = True if options.python and ast is None: parser.error("The ast module is required to read python syntax") - if options.json_output and json is None: - parser.error("The json module is required to output JSON syntax") - activate_session(session, goptions) + if (options.json_output or options.json_input) and json is None: + parser.error("The json module is required to use JSON syntax") + + def parse_arg(arg): + try: + if options.python: + return ast.literal_eval(arg) + elif options.json_input: + return json.loads(arg) + else: + return arg_filter(arg) + except ValueError as e: + if options.bare_strings: + return arg + else: + parser.error(f"Invalid value: {arg!r}") + + # the method to call name = args[0] - non_kw = [] + + # base kw args + # we update with name=value args later kw = {} - if options.python: - non_kw = [ast.literal_eval(a) for a in args[1:]] - if options.kwargs: - kw = ast.literal_eval(options.kwargs) - else: - for arg in args[1:]: - if arg.find('=') != -1: - key, value = arg.split('=', 1) - kw[key] = arg_filter(value) - else: - non_kw.append(arg_filter(arg)) + if options.kwargs: + kw = parse_arg(options.kwargs) + + # read the args + non_kw = [] + for arg in args[1:]: + if arg.find('=') != -1: + key, value = arg.split('=', 1) + kw[key] = parse_arg(value) + else: + non_kw.append(parse_arg(arg)) + + # make the call + activate_session(session, goptions) response = getattr(session, name).__call__(*non_kw, **kw) + + # print the result if options.json_output: print(json.dumps(response, indent=2, separators=(',', ': '), cls=DatetimeJSONEncoder)) else: From ab8ddbd1b09bc375c54e6cd583c56952c0b69b60 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 15 2023 03:59:27 +0000 Subject: [PATCH 2/5] unit tests --- diff --git a/tests/test_cli/test_call.py b/tests/test_cli/test_call.py index 52c2ea4..9c89b04 100644 --- a/tests/test_cli/test_call.py +++ b/tests/test_cli/test_call.py @@ -97,7 +97,7 @@ Note, that you can use global option --noauth for anonymous calls here module = { 'ast': "The ast module is required to read python syntax", - 'json': "The json module is required to output JSON syntax", + 'json': "The json module is required to use JSON syntax", } for mod, msg in module.items(): @@ -119,11 +119,14 @@ Note, that you can use global option --noauth for anonymous calls here (Specify the --help global option for a list of other help options) Options: - -h, --help show this help message and exit - --python Use python syntax for RPC parameter values - --kwargs=KWARGS Specify keyword arguments as a dictionary (implies - --python) - --json-output Use JSON syntax for output + -h, --help show this help message and exit + -p, --python Use python syntax for RPC parameter values + --kwargs=KWARGS Specify keyword arguments as a dictionary (implies + --python or --json-input) + -j, --json Use JSON syntax for input and output + --json-input Use JSON syntax for input + --json-output Use JSON syntax for output + -b, --bare-strings Treat invalid json/python as bare strings """ % self.progname) From 9121dd97a84933bd3796634f7195086ccee5428b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 15 2023 03:59:27 +0000 Subject: [PATCH 3/5] additional unit tests --- diff --git a/tests/test_cli/test_call.py b/tests/test_cli/test_call.py index 9c89b04..aca2ca3 100644 --- a/tests/test_cli/test_call.py +++ b/tests/test_cli/test_call.py @@ -58,6 +58,49 @@ Note, that you can use global option --noauth for anonymous calls here self.assert_console_message(stdout, "'%s'\n" % response[1]) @mock.patch('sys.stdout', new_callable=six.StringIO) + def test_handle_call_json_syntax(self, stdout): + """Test handle_call with json input syntax""" + response = ["SUCCESS", "FAKE-RESPONSE"] + self.session.ssl_login.return_value = response[1] + + # Invalid json syntax + arguments = ['--json-input', 'ssl_login', 'cert=/etc/pki/cert'] + self.assert_system_exit( + handle_call, + self.options, self.session, arguments, + stderr=self.format_error_message("Invalid value: '/etc/pki/cert'"), + activate_session=None) + self.activate_session_mock.assert_not_called() + + # Incompatible opts + arguments = ['--json', '--python', 'ssl_login', 'cert=/etc/pki/cert'] + self.assert_system_exit( + handle_call, + self.options, self.session, arguments, + stderr=self.format_error_message("The --python option conflicts with using --json-input"), + activate_session=None) + self.activate_session_mock.assert_not_called() + + arguments = ['--json-input', 'ssl_login', '--kwargs', '{"cert":"/etc/pki/cert"}'] + handle_call(self.options, self.session, arguments) + self.activate_session_mock.assert_called_with(self.session, self.options) + self.session.ssl_login.assert_called_with(cert='/etc/pki/cert') + self.assert_console_message(stdout, "'%s'\n" % response[1]) + + @mock.patch('sys.stdout', new_callable=six.StringIO) + def test_handle_call_bare_strings(self, stdout): + """Test handle_call with bare string fallback""" + response = ["SUCCESS", "FAKE-RESPONSE"] + self.session.ssl_login.return_value = response[1] + + # Invalid json syntax, but with bare-string fallback + arguments = ['--json-input', '--bare-strings', 'ssl_login', 'cert=/etc/pki/cert'] + handle_call(self.options, self.session, arguments) + self.activate_session_mock.assert_called_with(self.session, self.options) + self.session.ssl_login.assert_called_with(cert='/etc/pki/cert') + self.assert_console_message(stdout, "'%s'\n" % response[1]) + + @mock.patch('sys.stdout', new_callable=six.StringIO) def test_handle_call_json_output(self, stdout): """Test handle_call with json output""" arguments = ['ssl_login', 'cert=/etc/pki/cert', '--json-output'] From 1e71e0e0a2cd67e6aa1c3f4d95b2810d3c35fd82 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 15 2023 14:16:45 +0000 Subject: [PATCH 4/5] be pickier about keyword args and add a note about them in usage --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 59a2594..7604460 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -963,6 +963,7 @@ def handle_call(goptions, session, args): usage = """\ usage: %prog call [options] [ ...] + values of the form NAME=VALUE are treated as keyword arguments Note, that you can use global option --noauth for anonymous calls here""" usage = textwrap.dedent(usage) parser = OptionParser(usage=get_usage_str(usage)) @@ -1016,11 +1017,14 @@ def handle_call(goptions, session, args): if options.kwargs: kw = parse_arg(options.kwargs) + kw_pat = re.compile(r'^([^\W0-9]\w*)=(.*)$') + # read the args non_kw = [] for arg in args[1:]: - if arg.find('=') != -1: - key, value = arg.split('=', 1) + m = kw_pat.match(arg) + if m: + key, value = m.groups() kw[key] = parse_arg(value) else: non_kw.append(parse_arg(arg)) diff --git a/tests/test_cli/test_call.py b/tests/test_cli/test_call.py index aca2ca3..727fefc 100644 --- a/tests/test_cli/test_call.py +++ b/tests/test_cli/test_call.py @@ -22,6 +22,7 @@ class TestCall(utils.CliTestCase): self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start() self.error_format = """Usage: %s call [options] [ ...] + values of the form NAME=VALUE are treated as keyword arguments Note, that you can use global option --noauth for anonymous calls here (Specify the --help global option for a list of other help options) @@ -158,6 +159,7 @@ Note, that you can use global option --noauth for anonymous calls here handle_call, """Usage: %s call [options] [ ...] + values of the form NAME=VALUE are treated as keyword arguments Note, that you can use global option --noauth for anonymous calls here (Specify the --help global option for a list of other help options) From 766973ab3e101cefd7afe1da4506a871706064c6 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 26 2023 14:37:50 +0000 Subject: [PATCH 5/5] avoid using fstrings in client code --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 7604460..cc694e4 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1006,7 +1006,7 @@ def handle_call(goptions, session, args): if options.bare_strings: return arg else: - parser.error(f"Invalid value: {arg!r}") + parser.error("Invalid value: %r" % arg) # the method to call name = args[0]