#5 Port to Python 3
Merged 6 years ago by till. Opened 6 years ago by pviktori.
pviktori/fedora-easy-karma py3  into  master

file modified
+47 -44
@@ -1,4 +1,4 @@ 

- #!/usr/bin/python2

+ #!/usr/bin/python3

  # vim: fileencoding=utf8

  # Copyright 2010-2017 Till Maas and others

  # This file is part of fedora-easy-karma.
@@ -16,8 +16,10 @@ 

  # You should have received a copy of the GNU General Public License

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

  

+ from __future__ import print_function

+ 

  # default python modules

- import cPickle as pickle

+ import pickle

  import datetime

  import fnmatch

  import getpass
@@ -59,16 +61,18 @@ 

  PROMPT = "Comment? -1/0/1 -> karma, 'i' -> ignore, other -> skip> "

  

  

- def munch_to_dict(munch):

-     """ Recursively convert Munch class used in Bodhi2 to dict for easy pretty

-     printing

-     """

-     if str(munch.__class__) == "<class 'munch.Munch'>":

-         return dict([(i[0], munch_to_dict(i[1])) for i in munch.items()])

-     elif munch.__class__ == list:

-         return [munch_to_dict(i) for i in munch]

-     else:

-         return munch

+ # Use Python 2 names of things

+ try:

+     basestring

+ except NameError:

+     # Python 3 -- unicode strings only

+     basestring = str

+ 

+ try:

+     raw_input

+ except NameError:

+     # Python 3 -- raw_input() was renamed to input()

+     raw_input = input

  

  

  class FEK_helper(object):
@@ -225,15 +229,15 @@ 

                          subsequent_indent=(" " * 11 + ": "),

                          second_column_indent=0):

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

-             map(lambda p: "\n".join(

-                 wrap(p,

-                      width=width,

-                      subsequent_indent=(

-                          subsequent_indent + " " * second_column_indent)

-                      )

-                 ),

-                 paragraphs

+             "\n".join(

+                 wrap(

+                     p,

+                     width=width,

+                     subsequent_indent=(

+                         subsequent_indent + " " * second_column_indent)

                  )

+             )

+             for p in paragraphs

          )

  

      @staticmethod
@@ -501,11 +505,11 @@ 

          if self.options.product == "F":

              release_filename = "/etc/fedora-release"

              try:

-                 with open(release_filename, "rb") as release_file:

+                 with open(release_filename, "r") as release_file:

                      if "Rawhide" in release_file.read():

-                         print "'Rawhide' found in %s, aborting, because "\

+                         print("'Rawhide' found in %s, aborting, because "\

                                "there is no updates-testing for "\

-                               "Rawhide" % release_filename

+                               "Rawhide" % release_filename)

                          sys.exit(1)

              except IOError:

                  self.warning("Cannot read '%s', this system might not be "
@@ -569,8 +573,8 @@ 

                  cachefile = open(cachefile_name, "rb")

                  testing_updates = pickle.load(cachefile)

                  cachefile.close()

-             except IOError, ioe:

-                 print "Cannot access bodhi cache file: %s" % cachefile_name

+             except IOError as ioe:

+                 print("Cannot access bodhi cache file: %s" % cachefile_name)

                  sys.exit(ioe.errno)

          else:

              self.info("Waiting for Bodhi for a list of packages in "
@@ -607,7 +611,7 @@ 

              ignorefile = open(ignorefile_name, "rb")

              previously_ignored_updates = pickle.load(ignorefile)

              ignorefile.close()

-         except IOError, ioe:

+         except IOError as ioe:

              self.debug("Cannot access ignore file: %s" % ignorefile_name)

  

          self.debug("post processing bodhi query")
@@ -630,13 +634,12 @@ 

          # per update

          processed_updates = []

          ignored_updates = []

-         builds = testing_builds.keys()

-         builds.sort()

+         builds = sorted(testing_builds)

  

          if not builds:

-             print "No testing packages found, install some with: "\

+             print("No testing packages found, install some with: "\

                  "'%s update --enablerepo=\"*-testing\"'" % \

-                 pkghelper.package_manager

+                 pkghelper.package_manager)

          for build in builds:

              update = testing_builds[build]

  
@@ -644,7 +647,7 @@ 

              # Store update title to save these to a file

              if not self.options.include_ignored and \

                      update.title in previously_ignored_updates:

-                 print "ignored: %s" % update.title

+                 print("ignored: %s" % update.title)

                  ignored_updates.append(update.title)

                  continue

  
@@ -679,22 +682,22 @@ 

                      import ipdb

                      ipdb.set_trace()

                  if not self.options.list_rpms_only:

-                     print FEK_helper.bodhi_update_str(

+                     print(FEK_helper.bodhi_update_str(

                          update, bodhi_base_url=bc.base_url,

                          width=self.options.wrap_width,

                          wrap_bugs=self.options.wrap_bugs

-                     )

+                     ))

                      if self.options.wrap_rpms:

-                         print FEK_helper.wrap_paragraphs_prefix(

+                         print(FEK_helper.wrap_paragraphs_prefix(

                              installed_rpms, first_prefix=" inst. RPMS: ",

-                             width=self.options.wrap_width)

+                             width=self.options.wrap_width))

                      else:

                          indentation = "\n" + " " * 11 + ": "

                          rpmlist = indentation.join(installed_rpms)

-                         print " inst. RPMS: %s\n" % rpmlist

+                         print(" inst. RPMS: %s\n" % rpmlist)

                      if self.already_commented(update,

                                                self.options.fas_username):

-                         print "!!! already commented by you !!!"

+                         print("!!! already commented by you !!!")

                      try:

                          karma = self.raw_input(

                              PROMPT, default=self.options.default_karma,
@@ -710,17 +713,17 @@ 

                                      self.warning("Comment not submitted: %s" %

                                                   result[1])

                              else:

-                                 print "skipped because of empty comment"

+                                 print("skipped because of empty comment")

                          elif karma == "i":

                              ignored_updates.append(update.title)

-                             print "ignored as requested"

+                             print("ignored as requested")

  

                      except EOFError:

                          ignored_updates.extend(previously_ignored_updates)

                          sys.stdout.write("\nExiting on User request\n")

                          break

                  else:

-                     print "\n".join(installed_rpms)

+                     print("\n".join(installed_rpms))

  

          # store ignored_updates

          try:
@@ -763,7 +766,7 @@ 

          # There is no clear indication which Exceptions bc.query() might

          # throw, therefore catch all (python-fedora-0.3.32.3-1.fc19)

          except Exception as e:

-             print "Error while querying Bodhi: {0}".format(e)

+             print("Error while querying Bodhi: {0}".format(e))

              raise e

  

          return updates
@@ -832,11 +835,11 @@ 

                  res = bc.comment(update["title"], comment, karma=karma)

                  bc.retries = orig_retries

                  return (True, res)

-             except fedora.client.AuthError, e:

+             except fedora.client.AuthError as e:

                  self.warning("Authentication error")

                  bc.password = getpass.getpass('FAS password for %s: ' %

                                                self.options.fas_username)

-             except fedora.client.ServerError, e:

+             except fedora.client.ServerError as e:

                  self.warning("Server error: %s" % str(e))

  

          bc.retries = orig_retries
@@ -847,5 +850,5 @@ 

      try:

          fek = FedoraEasyKarma()

      except KeyboardInterrupt:

-         print "aborted"

+         print("aborted")

          sys.exit(0)

Here's ported to Python 3.
I gave karma using this code on Python 3 (https://bodhi.fedoraproject.org/updates/FEDORA-2018-abce118ab9), but I don't see tests and don't really know all the features and edge cases to test. Hopefully, there is someone better to do that!

I did a review of all changes/commits one by one and a lot of tests with Python 3 using various command line options and arguments. I gave positive karma to two updates:

Everything looks good to me.

I have only one nitpick. When you use FEK with Python 3, it'll use the highest pickle protocol - 4. When you then use FEK with Python 2, it will be unable to read the cache files because Python 2 doesn't support pickle protocol 4. This is not an issue when you use FEK with only one Python version all the time. This corner case might be fixed by specifying the pickle protocol supported in both Pythons - version 2.

$ python2 ~/temp/fedora-easy-karma.py --list-rpms-only
Getting list of installed packages...
Waiting for Bodhi for a list of packages in updates-testing (F29)...
Fetching updates page 2 of 14
Fetching updates page 3 of 14
Fetching updates page 4 of 14
Fetching updates page 5 of 14
Fetching updates page 6 of 14
Fetching updates page 7 of 14
Fetching updates page 8 of 14
Fetching updates page 9 of 14
Fetching updates page 10 of 14
Fetching updates page 11 of 14
Fetching updates page 12 of 14
Fetching updates page 13 of 14
Fetching updates page 14 of 14
found 334 testing updates
Traceback (most recent call last):
  File "/home/lbalhar/temp/fedora-easy-karma.py", line 851, in <module>
    fek = FedoraEasyKarma()
  File "/home/lbalhar/temp/fedora-easy-karma.py", line 612, in __init__
    previously_ignored_updates = pickle.load(ignorefile)
  File "/usr/lib64/python2.7/pickle.py", line 1384, in load
    return Unpickler(file).load()
  File "/usr/lib64/python2.7/pickle.py", line 864, in load
    dispatch[key](self)
  File "/usr/lib64/python2.7/pickle.py", line 892, in load_proto
    raise ValueError, "unsupported pickle protocol: %d" % proto
ValueError: unsupported pickle protocol: 4

I tend to prefer importing unicode strings from the future so both the Py 2 and Py 3 versions behave the same wrt strings, but it's kinda a personal preference thing, I guess.

Aside from that, this looks good to me. There's lots of other junk in here we could clean up, and we should really write some tests, but on an eyeball review I don't see any missing conversions.

Thank you very much! I am not sure if the pickle file problem is a realistic issue since it would happen when someone uses the same cache/ignore file for Fedora and EPEL but then the caches woukl conain the wrong data. What do you think?

The pickle format should depend not on the repos you're working with, but on which Python you run fedora-easy-karma with.
So, this would become a problem if you use FEK with py3, but then downgrade (or run it with Python 2 manually). Or if you share the cache files across machines with different f-e-k releases. To me, neither looks like something to worry about.

Given the f-e-k is used on a single machine to send karma for updates on that machine, I think we don't need to worry about pickle compatibility between Python 2 and 3.

Pull-Request has been merged by till

6 years ago
Metadata