#45 Change optparse with argparse
Merged 2 months ago by lruzicka. Opened 2 months ago by lruzicka.

file modified
+62 -93
@@ -17,6 +17,7 @@ 

  # along with fedora-easy-karma.  If not, see <http://www.gnu.org/licenses/>.

  

  # standard python modules

+ import argparse

  import datetime

  import fnmatch

  import itertools
@@ -31,7 +32,6 @@ 

  import colored

  import dnf

  import munch

- from optparse import OptionParser

  import requests

  from textwrap import wrap

  
@@ -274,38 +274,30 @@ 

  

          return ("\n%s" % subsequent_indent).join(output)

  

- USAGE = """usage: %prog [options] [pattern, ..]

- 

- You will be asked for every package installed from updates-testing to provide

- feedback using karma points.  If patterns are provided, you will be only

- prompted for updates related to packages or builds that match any of the

- patterns. Possible wildcards are *, ?, [seq] and [!seq] as explained at

- http://docs.python.org/library/fnmatch.html

+ USAGE = """

+ You will be asked for every package installed from updates-testing to

+ provide feedback using karma points. If patterns are provided, you will be

+ only prompted for updates related to packages or builds that match any of

+ the patterns.  Possible wildcards are *, ?, [seq] and [!seq]. More is about

+ the wildcards is explained at <http://docs.python.org/library/fnmatch.html>.

  

  Possible values in the karma prompt:

      -1,0 or 1: Assign the respective karma value to the update

-     i: Ignore the update in the future

-     Other inputs will skip the update.

- 

- After assigning karma to the update, a comment needs to be provided, otherwise

- the update will be skipped.

+      i: Ignore the update in the future

+      Other inputs will skip the update.

  

  Note:

- <CTRL>-<D> on an empty prompt exits the program.

- If you use a default comment, '<CTRL>-<X> <backspace>' can be used to delete

- the default comment to easily enter a custom one.

+     * <CTRL>-<D> exits the program when used on empty prompt.

+     * <CTRL>-<X> + <backspace> deletes the default comment to enter a new one.

  

  For further documentation, please visit:

  https://fedoraproject.org/wiki/Fedora_Easy_Karma

  

- Copyright 2010-2017 Till Maas and others

- fedora-easy-karma is distributed under the terms of the GNU General Public

- License

- The source is available at:

- https://pagure.io/fedora-easy-karma

+ Copyright 2010-2025 Till Maas and others. Fedora-easy-karma is distributed under

+ the terms of the GNU General Public License. The source is available at

+ <https://pagure.io/fedora-easy-karma>.

  """

  

- 

  class PkgHelper(object):

      def __init__(self):

          self.my = dnf.Base()
@@ -332,175 +324,152 @@ 

              width=80,

              extra_newline=False)

  

-         parser = OptionParser(usage=usage)

-         parser.add_option("",

-                           "--bodhi-cached", dest="bodhi_cached",

+         parser = argparse.ArgumentParser(description=usage, formatter_class=argparse.RawDescriptionHelpFormatter)

+         parser.add_argument("--bodhi-cached", dest="bodhi_cached",

                            help="Use cached bodhi query",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--bodhi-update-cache",

+         parser.add_argument("--bodhi-update-cache",

                            dest="bodhi_update_cache",

                            help="Update bodhi query cache",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--critpath-only",

+         parser.add_argument("--critpath-only",

                            dest="critpath_only",

                            help="Only consider unapproved critpath updates",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--datadir",

+         parser.add_argument("--datadir",

                            dest="datadir",

                            help="Directory to store cache or ignore data, "

-                                "default: %default",

+                                "default: %(default)s",

                            default=f"{config_dir}/fedora-easy-karma")

-         parser.add_option("",

-                           "--debug",

+         parser.add_argument("--debug",

                            dest="debug",

                            help="Enable debug output",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--default-comment",

+         parser.add_argument("--default-comment",

                            dest="default_comment",

-                           help="Default comment to use, default: %default",

+                           help="Default comment to use, default: %(default)s",

                            default="",

                            metavar="COMMENT")

-         parser.add_option("",

-                           "--default-karma",

+         parser.add_argument("--default-karma",

                            dest="default_karma",

-                           help="Default karma to use, default: %default",

+                           help="Default karma to use, default: %(default)s",

                            default="",

                            metavar="KARMA")

-         parser.add_option("",

-                           "--no-color",

+         parser.add_argument("--no-color",

                            dest="no_color",

                            help="Do not colorize output",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--no-ignore-own",

+         parser.add_argument("--no-ignore-own",

                            dest="ignore_own",

                            help="Do not ignore own updates.",

                            action="store_false",

                            default=True)

-         parser.add_option("",

-                           "--include-commented",

+         parser.add_argument("--include-commented",

                            dest="include_commented",

                            help="Also ask for more comments on updates that "

                                 "already got a comment from you, this is "

                                 "enabled if patterns are provided",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--include-ignored",

+         parser.add_argument("--include-ignored",

                            dest="include_ignored",

                            help="Also ask for comments on updates that have "

                                 "been ignored previously.",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--installed-max-days",

+         parser.add_argument("--installed-max-days",

                            dest="installed_max_days",

                            help="Only check packages installed within the last "

-                                "XX days, default: %default",

+                                "XX days, default: %(default)d",

                            metavar="DAYS",

                            default=28,

-                           type="int")

-         parser.add_option("",

-                           "--installed-min-days",

+                           type=int)

+         parser.add_argument("--installed-min-days",

                            dest="installed_min_days",

                            help="Only check packages installed for at least "

-                                "XX days, default: %default",

+                                "XX days, default: %(default)d",

                            metavar="DAYS",

                            default=0,

-                           type="int")

-         parser.add_option("",

-                           "--ipdb",

+                           type=int)

+         parser.add_argument("--ipdb",

                            dest="ipdb",

                            help="Launch ipbd for debugging",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--list-rpms-only",

+         parser.add_argument("--list-rpms-only",

                            dest="list_rpms_only",

                            help="Only list affected rpms",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--pages",

+         parser.add_argument("--pages",

                            dest="pages",

                            help="Clear terminal before each new presented update",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--product",

+         parser.add_argument("--product",

                            dest="product",

                            help="product to query Bodhi for, 'F' for Fedora, "

-                                "'EL-' for EPEL, default: %default",

+                                "'EL-' for EPEL, default: %(default)s",

                            default="F")

-         parser.add_option("",

-                           "--releasever",

+         parser.add_argument("--releasever",

                            dest="releasever",

                            help="releasever to query Bodhi for, "

                                 "default: releasever from dnf",

                            default=None)

-         parser.add_option("",

-                           "--retries",

+         parser.add_argument("--retries",

                            dest="retries",

                            help="Number if retries when submitting a comment "

-                                "in case of an error, default: %default",

+                                "in case of an error, default: %(default)d",

                            default=3,

-                           type="int")

-         parser.add_option("",

-                           "--skip-bodhi-comments",

+                           type=int)

+         parser.add_argument("--skip-bodhi-comments",

                            dest="skip_bodhi_comments",

                            help="Do not show comments created by bodhi",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--wrap-bugs",

+         parser.add_argument("--wrap-bugs",

                            dest="wrap_bugs",

                            help="Apply line-wrapping to bugs",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--wrap-rpms",

+         parser.add_argument("--wrap-rpms",

                            dest="wrap_rpms",

                            help="Apply line-wrapping to list of installed rpms",

                            action="store_true",

                            default=False)

-         parser.add_option("",

-                           "--wrap-width",

+         parser.add_argument("--wrap-width",

                            dest="wrap_width",

                            help="Width to use for line wrapping of updates, "

-                                "default: %default",

+                                "default: %(default)d",

                            default=80,

-                           type="int")

-         parser.add_option("",

-                           "--bodhi-request-limit",

+                           type=int)

+         parser.add_argument("--bodhi-request-limit",

                            dest="bodhi_request_limit",

                            help="Maximum number of updates to request at "

-                           "once from Bodhi, default: %default",

+                           "once from Bodhi, default: %(default)d",

                            default=25,

-                           type="int")

-         parser.add_option("",

-                           "--oraculum-endpoint",

+                           type=int)

+         parser.add_argument("--oraculum-endpoint",

                            dest="oraculum_endpoint",

                            help="Specify URL for oraculum instance",

                            default="https://packager-dashboard.fedoraproject.org/api/v1/libkarma/")

-         parser.add_option("",

-                           "--no-cache",

+         parser.add_argument("--no-cache",

                            dest="oraculum_disabled",

                            help="Bypass oraculum and force fetch from bodhi",

                            action="store_true",

                            default=False)

+         parser.add_argument("pattern",

+                           help="Show only updates matching one or more package name patterns",

+                           nargs="*")

  

-         (self.options, args) = parser.parse_args()

+         self.options = parser.parse_args()

  

-         if args:

+         if self.options.pattern:

              self.options.include_commented = True

  

          if self.options.debug:
@@ -689,13 +658,13 @@ 

                                        b in installed_testing_builds])

                  )

  

-                 if args:

+                 if self.options.pattern:

                      installed_pkgs_names = ["%(name)s" % pkg for pkg in

                                              installed_pkgs]

                      # remove version and release

                      affected_builds_names = ["-".join(b.split("-")[:-2]) for b

                                               in affected_builds]

-                     if not self.match_any(args, [installed_pkgs_names,

+                     if not self.match_any(self.options.pattern, [installed_pkgs_names,

                                                   affected_builds_names]):

                          continue

                  installed_rpms = [

The previous version used the optparse module that has been deprecated
for some time already and Pylint would constantly warn about it.
The PR replaces optparse with argparse while retaining the logic
to add searching patterns on the CLI, similarly to other CLI tools,
such as ls, etc.

Fixes: https://pagure.io/fedora-easy-karma/issue/41

rebased onto 6d931dd

2 months ago

rebased onto 57e470e

2 months ago

Hmm, it seems to be crashing for me, can you check?
(Please note that I added one more commit to master and rebased your PR, so you need to pull it and then git reset --hard origin/fix/argparse to switch to it. But the code is crashing for me even in your original commit b540691 , so the rebase is not the cause of the problem).

$ python fedora-easy-karma.py --help
Traceback (most recent call last):
  File "/home/kparal/devel/qa/fedora-easy-karma/fedora-easy-karma.py", line 900, in <module>
    fek = FedoraEasyKarma()
  File "/home/kparal/devel/qa/fedora-easy-karma/fedora-easy-karma.py", line 476, in __init__
    self.options = parser.parse_args()
                   ~~~~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/argparse.py", line 1889, in parse_args
    args, argv = self.parse_known_args(args, namespace)
                 ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 1899, in parse_known_args
    return self._parse_known_args2(args, namespace, intermixed=False)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 1928, in _parse_known_args2
    namespace, args = self._parse_known_args(args, namespace, intermixed)
                      ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 2179, in _parse_known_args
    start_index = consume_optional(start_index)
  File "/usr/lib64/python3.13/argparse.py", line 2103, in consume_optional
    take_action(action, args, option_string)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 2004, in take_action
    action(self, namespace, argument_values, option_string)
    ~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 1121, in __call__
    parser.print_help()
    ~~~~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/argparse.py", line 2616, in print_help
    self._print_message(self.format_help(), file)
                        ~~~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/argparse.py", line 2600, in format_help
    return formatter.format_help()
           ~~~~~~~~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/argparse.py", line 284, in format_help
    help = self._root_section.format_help()
  File "/usr/lib64/python3.13/argparse.py", line 215, in format_help
    item_help = join([func(*args) for func, args in self.items])
                      ~~~~^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 215, in format_help
    item_help = join([func(*args) for func, args in self.items])
                      ~~~~^^^^^^^
  File "/usr/lib64/python3.13/argparse.py", line 509, in _format_action
    help_text = self._expand_help(action)
  File "/usr/lib64/python3.13/argparse.py", line 599, in _expand_help
    return self._get_help_string(action) % params
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
TypeError: %d format: a real number is required, not dict

rebased onto 57e470e

2 months ago

The %default placeholders were not compatible with argparse, so I used the compatible variants %(default)s and %(default)d. The --help argument does not cause the app to crash any more.

I also cleaned the USAGE string a little bit. I am not convinced about it much.

To clarify,
I am not convinced that the USAGE string should look like it looks at the moment. Maybe, some more tweaking could be done to it, but this is not a matter of this change, so be it.

This could help keep the newlines in the usage string:

    parser = argparse.ArgumentParser(
        formatter_class=argparse.RawDescriptionHelpFormatter,
        description=...

2 new commits added

  • Fix crashing when --help called.
  • Change optparse with argparse
2 months ago

Yes, I used the above to force the formatting of the USAGE output.

rebased onto 6703c50

2 months ago

3 new commits added

  • extras -> pattern
  • reflow the usage text a bit better
  • probably wrongly-merged rebase
2 months ago

I made some small tweaks. If you're OK with the changes, feel free to squash it into a single commit and push it. Or I can do it, if you prefer.

I made some small tweaks. If you're OK with the changes, feel free to squash it into a single commit and push it. Or I can do it, if you prefer.

I can do it.

rebased onto 6703c50

2 months ago

rebased onto 6703c50

2 months ago

Commit f596ef4 fixes this pull-request

Pull-Request has been merged by lruzicka

2 months ago

Thanks. Just please don't forget to comb the resulting commit message a bit next time, there's no point in keeping all intermediary commit messages in there (written just for the purpose of the PR process) :-)

Thanks. Just please don't forget to comb the resulting commit message a bit next time, there's no point in keeping all intermediary commit messages in there (written just for the purpose of the PR process) :-)

I usually delete them, but this time I had a feeling you would have not wanted them to be deleted.

Ideally the commit message is massaged into a form that makes sense for anyone reading the git history and displaying this exact commit. So if anything is no longer true in the very first commit message, of course it should be amended. But there's no point in keeping "change the name of a variable" or "fix crash" or "reflow text" messages in there, because the person reading the history doesn't see any of it in the final code, and they don't even care (the final result is what matters, not the whole PR process - for that, this ticket can be studied).

Metadata