PR#3740: ArgumentParser instead of ArgumentParser in sidetag CLI plugin - koji

koji

#3740 ArgumentParser instead of ArgumentParser in sidetag CLI plugin

Merged a year ago by tkopecek. Opened a year ago by jcupova.

jcupova/koji issue-3739 into master

ArgumentParser instead of ArgumentParser in sidetag CLI plugin

Jana Cupova • a year ago

1b18593

plugins/cli/sidetag_cli.py

file modified

+47 -36

		`@@ -5,7 +5,7 @@`

		`from __future__ import absolute_import`

		`- from argparse import ArgumentParser`
		`+ from optparse import OptionParser`

		`import koji`
		`from koji.plugin import export_cli`
		`@@ -16,18 +16,21 @@`
		`@export_cli`
		`def handle_add_sidetag(options, session, args):`
		`"Create sidetag"`
		`- usage = "%(prog)s add-sidetag [options] <basetag>"`
		`+ usage = "%prog add-sidetag [options] <basetag>"`
		`usage += "\n(Specify the --help global option for a list of other help options)"`
		`- parser = ArgumentParser(usage=usage)`
		`- parser.add_argument("basetag", help="name of basetag")`
		`- parser.add_argument("-q", "--quiet", action="store_true", help="Do not print tag name",`
		`- default=options.quiet)`
		`- parser.add_argument("-w", "--wait", action="store_true", help="Wait until repo is ready.")`
		`- parser.add_argument("--debuginfo", action="store_true",`
		`- help="Buildroot repo will contain debuginfos")`
		`- parser.add_argument("--suffix", action="store", help="Suffix from hub-supported ones")`
		`+ parser = OptionParser(usage=usage)`
		`+ parser.add_option("-q", "--quiet", action="store_true", help="Do not print tag name",`
		`+ default=options.quiet)`
		`+ parser.add_option("-w", "--wait", action="store_true", help="Wait until repo is ready.")`
		`+ parser.add_option("--debuginfo", action="store_true",`
		`+ help="Buildroot repo will contain debuginfos")`
		`+ parser.add_option("--suffix", action="store", help="Suffix from hub-supported ones")`

		`- opts = parser.parse_args(args)`
		`+ (opts, args) = parser.parse_args(args)`
		`+`
		`+ if len(args) != 1:`
		`+ parser.error("Only argument is basetag")`
		`+ basetag = args[0]`

		`activate_session(session, options)`

		`@@ -35,7 +38,7 @@`
		`if opts.suffix:`
		`kwargs['suffix'] = opts.suffix`
		`try:`
		`- tag = session.createSideTag(opts.basetag, **kwargs)`
		`+ tag = session.createSideTag(basetag, **kwargs)`
		`except koji.ActionNotAllowed:`
		`parser.error("Policy violation")`
		`except koji.ParameterError as ex:`
		`@@ -57,16 +60,18 @@`
		`@export_cli`
		`def handle_remove_sidetag(options, session, args):`
		`"Remove sidetag"`
		`- usage = "%(prog)s remove-sidetag [options] <sidetag> ..."`
		`+ usage = "%prog remove-sidetag [options] <sidetag> ..."`
		`usage += "\n(Specify the --help global option for a list of other help options)"`
		`- parser = ArgumentParser(usage=usage)`
		`- parser.add_argument("sidetags", help="name of sidetag", nargs="+")`
		`- opts = parser.parse_args(args)`
		`+ parser = OptionParser(usage=usage)`
		`+ (opts, args) = parser.parse_args(args)`
		`+`
		`+ if len(args) < 1:`
		`+ parser.error("Sidetag argument is required")`

		`activate_session(session, options)`

		`session.multicall = True`
		`- for sidetag in opts.sidetags:`
		`+ for sidetag in args:`
		`session.removeSideTag(sidetag)`
		`session.multiCall(strict=True)`

		`@@ -74,14 +79,17 @@`
		`@export_cli`
		`def handle_list_sidetags(options, session, args):`
		`"List sidetags"`
		`- usage = "%(prog)s list-sidetags [options]"`
		`+ usage = "%prog list-sidetags [options]"`
		`usage += "\n(Specify the --help global option for a list of other help options)"`
		`- parser = ArgumentParser(usage=usage)`
		`- parser.add_argument("--basetag", action="store", help="Filter on basetag")`
		`- parser.add_argument("--user", action="store", help="Filter on user")`
		`- parser.add_argument("--mine", action="store_true", help="Filter on user")`
		`+ parser = OptionParser(usage=usage)`
		`+ parser.add_option("--basetag", action="store", help="Filter on basetag")`
		`+ parser.add_option("--user", action="store", help="Filter on user")`
		`+ parser.add_option("--mine", action="store_true", help="Filter on user")`
		`+`
		`+ (opts, args) = parser.parse_args(args)`

		`- opts = parser.parse_args(args)`
		`+ if len(args) > 0:`
		`+ parser.error("This command takes no arguments")`

		`if opts.mine and opts.user:`
		`parser.error("Specify only one from --user --mine")`
		`@@ -99,19 +107,22 @@`
		`@export_cli`
		`def handle_edit_sidetag(options, session, args):`
		`"Edit sidetag"`
		`- usage = "%(prog)s edit-sidetag [options] <sidetag>"`
		`+ usage = "%prog edit-sidetag [options] <sidetag>"`
		`usage += "\n(Specify the --help global option for a list of other help options)"`
		`- parser = ArgumentParser(usage=usage)`
		`- parser.add_argument("sidetag", help="name of sidetag")`
		`- parser.add_argument("--debuginfo", action="store_true", default=None,`
		`- help="Generate debuginfo repository")`
		`- parser.add_argument("--no-debuginfo", action="store_false", dest="debuginfo")`
		`- parser.add_argument("--rpm-macro", action="append", default=[], metavar="key=value",`
		`- dest="rpm_macros", help="Set tag-specific rpm macros")`
		`- parser.add_argument("--remove-rpm-macro", action="append", default=[], metavar="key",`
		`- dest="remove_rpm_macros", help="Remove rpm macros")`
		`-`
		`- opts = parser.parse_args(args)`
		`+ parser = OptionParser(usage=usage)`
		`+ parser.add_option("--debuginfo", action="store_true", default=None,`
		`+ help="Generate debuginfo repository")`
		`+ parser.add_option("--no-debuginfo", action="store_false", dest="debuginfo")`
		`+ parser.add_option("--rpm-macro", action="append", default=[], metavar="key=value",`
		`+ dest="rpm_macros", help="Set tag-specific rpm macros")`
		`+ parser.add_option("--remove-rpm-macro", action="append", default=[], metavar="key",`
		`+ dest="remove_rpm_macros", help="Remove rpm macros")`
		`+`
		`+ (opts, args) = parser.parse_args(args)`
		`+`
		`+ if len(args) != 1:`
		`+ parser.error("Only argument is sidetag")`
		`+ sidetag = args[0]`

		`if opts.debuginfo is None and not opts.rpm_macros and not opts.remove_rpm_macros:`
		`parser.error("At least one option needs to be specified")`
		`@@ -133,4 +144,4 @@`
		`if opts.remove_rpm_macros:`
		`kwargs['remove_rpm_macros'] = opts.remove_rpm_macros`

		`- session.editSideTag(opts.sidetag, **kwargs)`
		`+ session.editSideTag(sidetag, **kwargs)`

tests/test_plugins/test_sidetag_cli.py

file modified

+14 -23

		`@@ -50,18 +50,15 @@`
		`with self.assertRaises(SystemExit) as ex:`
		`sidetag.handle_add_sidetag(self.options, self.session, ['--help'])`
		`std_output = get_stdout_value(stdout).decode('utf-8')`
		`- expected_help = """usage: %s add-sidetag [options] <basetag>`
		`+ expected_help = """Usage: %s add-sidetag [options] <basetag>`
		`(Specify the --help global option for a list of other help options)`

		`- positional arguments:`
		`- basetag name of basetag`
		`-`
		`- options:`
		`+ Options:`
		`-h, --help show this help message and exit`
		`-q, --quiet Do not print tag name`
		`-w, --wait Wait until repo is ready.`
		`--debuginfo Buildroot repo will contain debuginfos`
		`- --suffix SUFFIX Suffix from hub-supported ones`
		`+ --suffix=SUFFIX Suffix from hub-supported ones`
		`""" % self.progname`
		`self.assertMultiLineEqual(std_output, expected_help)`
		`self.assertEqual('0', str(ex.exception))`
		`@@ -71,19 +68,16 @@`
		`with self.assertRaises(SystemExit) as ex:`
		`sidetag.handle_edit_sidetag(self.options, self.session, ['--help'])`
		`std_output = get_stdout_value(stdout).decode('utf-8')`
		`- expected_help = """usage: %s edit-sidetag [options] <sidetag>`
		`+ expected_help = """Usage: %s edit-sidetag [options] <sidetag>`
		`(Specify the --help global option for a list of other help options)`

		`- positional arguments:`
		`- sidetag name of sidetag`
		`-`
		`- options:`
		`+ Options:`
		`-h, --help show this help message and exit`
		`--debuginfo Generate debuginfo repository`
		`- --no-debuginfo`
		`- --rpm-macro key=value`
		`+ --no-debuginfo`
		`+ --rpm-macro=key=value`
		`Set tag-specific rpm macros`
		`- --remove-rpm-macro key`
		`+ --remove-rpm-macro=key`
		`Remove rpm macros`
		`""" % self.progname`
		`self.assertMultiLineEqual(std_output, expected_help)`
		`@@ -94,13 +88,13 @@`
		`with self.assertRaises(SystemExit) as ex:`
		`sidetag.handle_list_sidetags(self.options, self.session, ['--help'])`
		`std_output = get_stdout_value(stdout).decode('utf-8')`
		`- expected_help = """usage: %s list-sidetags [options]`
		`+ expected_help = """Usage: %s list-sidetags [options]`
		`(Specify the --help global option for a list of other help options)`

		`- options:`
		`+ Options:`
		`-h, --help show this help message and exit`
		`- --basetag BASETAG Filter on basetag`
		`- --user USER Filter on user`
		`+ --basetag=BASETAG Filter on basetag`
		`+ --user=USER Filter on user`
		`--mine Filter on user`
		`""" % self.progname`
		`self.assertMultiLineEqual(std_output, expected_help)`
		`@@ -111,13 +105,10 @@`
		`with self.assertRaises(SystemExit) as ex:`
		`sidetag.handle_remove_sidetag(self.options, self.session, ['--help'])`
		`std_output = get_stdout_value(stdout).decode('utf-8')`
		`- expected_help = """usage: %s remove-sidetag [options] <sidetag> ...`
		`+ expected_help = """Usage: %s remove-sidetag [options] <sidetag> ...`
		`(Specify the --help global option for a list of other help options)`

		`- positional arguments:`
		`- sidetags name of sidetag`
		`-`
		`- options:`
		`+ Options:`
		`-h, --help show this help message and exit`
		`""" % self.progname`
		`self.assertMultiLineEqual(std_output, expected_help)`

tox.ini

file modified

+2 -1

		`@@ -57,7 +57,8 @@`
		`tests/test_plugins/test_runroot_builder.py \`
		`tests/test_plugins/test_save_failed_tree_builder.py \`
		`tests/test_plugins/test_runroot_cli.py \`
		`- tests/test_plugins/test_save_failed_tree_cli.py}`
		`+ tests/test_plugins/test_save_failed_tree_cli.py \`
		`+ tests/test_plugins/test_sidetag_cli.py}`
		`{envbindir}/coverage2 report`
		`{envbindir}/coverage2 html -d {toxinidir}/htmlcov/py2`

jcupova commented a year ago

Fixes: https://pagure.io/koji/issue/3739

rebased onto 1b18593

a year ago

ignatenkobrain commented a year ago

Wouldn't it make more sense to port all other commands to ArgumentParser instead?

tkopecek commented a year ago

After RHEL6 retirement, yes (also dropping py2 support). For now we're going to unify these for easier testing. There were some subtle differences between output of the libraries.