fedpkg

Clone

#40 Allow fedpkg to be compatible with both bodhi 1 and bodhi 2.
Merged 7 years ago by cqi. Opened 7 years ago by bowlofeggs.
bowlofeggs/fedpkg use_bodhi_1_or_2  into  master

file modified
+27 -3 
@@ -16,6 +16,7 @@ 
 

  import re
 

  import fedora_cert
 

  import platform
 

+ import subprocess
 

  

  from .lookaside import FedoraLookasideCache
 

  from pyrpkg.utils import cached_property
 
@@ -304,9 +305,20 @@ 
 

      def update(self, template='bodhi.template', bugs=[]):
 

          """Submit an update to bodhi using the provided template."""
 

  

-         # build up the bodhi arguments
 

-         cmd = ['bodhi', 'updates', 'new', '--file', 'bodhi.template',
 

-                '--user', self.user, self.nvr]
 

+         # build up the bodhi arguments, based on which version of bodhi is
 

+         # installed
 

+         bodhi_major_version = _get_bodhi_version()[0]
 

+         if bodhi_major_version < 2:
 

+             cmd = ['bodhi', '--new', '--release', self.branch_merge,
 

+                    '--file', 'bodhi.template', self.nvr, '--username',
 

+                    self.user]
 

+         elif bodhi_major_version == 2:
 

+             cmd = ['bodhi', 'updates', 'new', '--file', 'bodhi.template',
 

+                    '--user', self.user, self.nvr]
 

+         else:
 

+             msg = 'This system has bodhi v{0}, which is unsupported.'
 

+             msg = msg.format(bodhi_major_version)
 

+             raise Exception(msg)
cqi commented 7 years ago

Better use pyrpkg.rpkgError instead of Exception

 
          self._run_command(cmd, shell=True)
 

  

      def load_kojisession(self, anon=False):
 
@@ -320,6 +332,18 @@ 
 

              raise
 

  

  

+ def _get_bodhi_version():
 

+     """
 

+     Use bodhi --version to determine the version of the Bodhi CLI that's
 

+     installed on the system, then return a list of the version components.
 

+     For example, if bodhi --version returns "2.1.9", this function will return
 

+     [2, 1, 9].
 

+     """
 

+     bodhi = subprocess.Popen(['bodhi', '--version'], stdout=subprocess.PIPE)

So what if fedpkg is updated but not the bodhi client? :)

bodhi 2.1.9 has never been released into any version of Fedora or EPEL yet. My original plan was to coordinate a release of fedpkg and bodhi to Rawhide in the same push, but the purpose of this patch is to make that unnecessary. This patch will make it possible (and the preferred path) for fedpkg to be updated in all platforms first, and then later release bodhi 2 into Rawhide and EPEL 7. At this time, I plan to leave bodhi at 0.9 in EPEL 6 and Fedoras 23-25, since it is a backwards incompatible update. I have permission from the EPEL steering committee to release the backwards incompatible update to EPEL 7.

 
+     version = bodhi.communicate()[0].strip()
 

+     return [int(component) for component in version.split('.')]
 

+ 

+ 

  if __name__ == "__main__":
 

      from fedpkg.__main__ import main
 

      main()

This commit modified fedpkg to detect the installed version of the
bodhi CLI, and then use the appropriate bodhi command to create
the update.

This will allow fedpkg to be installed on a variety of OS versions
with differing bodhi client versions.

Signed-off-by: Randy Barlow randy@electronsweatshop.com

This pull request adds the --version flag to the bodhi 2 CLI: https://github.com/fedora-infra/bodhi/pull/896

Would using pkg_resources be an option instead of shelling it out?

Something like https://github.com/fedora-infra/packagedb-cli/blob/master/pkgdb2client/__init__.py#L42 ?

So what if fedpkg is updated but not the bodhi client? :)

@pingou sure I can do that; I suppose it would be more efficient. I was wondering why this program shells out to bodhi in the first place instead of importing it, but I guess I was just following the pattern established here. I'll push up a new commit.

@pingou I looked into making that change, but I quickly came to realize that the bodhi 0.9 CLI wasn't installed with setuputils, but was just a script that was dropped into /usr/bin. Due to this, pkg_resources isn't able to detect its version. This also explains why fedpkg is shelling out to bodhi instead of importing it.

We could try pkg_resources and fall back to calling subprocess to use the --version flag, but I think I prefer the simplicity of having one consistent way of determining the version rather than a complex pattern.

What do you think?
Edited 7 years ago by bowlofeggs

bodhi 2.1.9 has never been released into any version of Fedora or EPEL yet. My original plan was to coordinate a release of fedpkg and bodhi to Rawhide in the same push, but the purpose of this patch is to make that unnecessary. This patch will make it possible (and the preferred path) for fedpkg to be updated in all platforms first, and then later release bodhi 2 into Rawhide and EPEL 7. At this time, I plan to leave bodhi at 0.9 in EPEL 6 and Fedoras 23-25, since it is a backwards incompatible update. I have permission from the EPEL steering committee to release the backwards incompatible update to EPEL 7.

@pingou I looked into making that change, but I quickly came to realize that the bodhi 0.9 CLI wasn't installed with setuputils, but was just a script that was dropped into /usr/bin. Due to this, pkg_resources isn't able to detect its version. This also explains why fedpkg is shelling out to bodhi instead of importing it.

I guess another reason is to hide bodhi details in fedpkg. /usr/bin/bodhi imports fedora.client.bodhi.BodhiClient and create instance of BodhiClient by passing Bodhi URL explicitly. At this moment, fedpkg does not need to maintain the Bodhi URL. Just subprocessing /usr/bin/bodhi could be a good way to hide this detail.

It would be nice to make bodhi be installed via setuptools (I suppose setuputils you mentioned above means setuptools), and then could be imported from fedpkg instead of calling subprocess.

We could try pkg_resources and fall back to calling subprocess to use the --version flag, but I think I prefer the simplicity of having one consistent way of determining the version rather than a complex pattern.

For the moment, it's okay by calling subprocess.
Edited 7 years ago by cqi

Better use pyrpkg.rpkgError instead of Exception

LGTM

On Wed, 2016-08-31 at 01:20 +0000, pagure@pagure.io wrote:

It would be nice to make bodhi be installed via setuptools (I suppose
setuputils you mentioned above means setuptools), and then could be
imported from fedpkg instead of calling subprocess.

Bodhi 2 is installed this way, but for now I am only planning to
release Bodhi 2 into Rawhide and EPEL 7. This means that fedpkg will
need to continue to be compatible with Bodhi 0.9 and 2.0, at least for
a while.

Thanks for your consideration!

Looks good.

Pull-Request has been merged by cqi
7 years ago

Merged. Thank you. @bowlofeggs when do you expect this change to be available in rawhide and EPEL?

Hello @cqi! I would like this change to be in Rawhide and EPEL 7 as soon as possible. I am working on getting Bodhi 2 into both of those and am blocked on fedpkg being compatible. I've filed BZs to request Bodhi 2 compatible fedpkg releases into Rawhide and EPEL 7:

https://bugzilla.redhat.com/show_bug.cgi?id=1371996
https://bugzilla.redhat.com/show_bug.cgi?id=1371998

Thanks, and let me know if I can help in any way!

On Wed, 2016-08-31 at 01:35 +0000, pagure@pagure.io wrote:

Better use pyrpkg.rpkgError instead of Exception

I'm happy to make this change if you like as a second pull request
(since this one is already merged). However, I will say that I did test
this error condition, and the user is presented with a clean looking
response (i.e., no traceback is printed and it looks nicely printed and
human friendly). Let me know if you want me to do this, and I'll be
happy to.

I don't think it's worth it to change the exception given that update method in cli.py that will be catching it doesn't really care.

@bowlofeggs Hi, as lsedlar mentioned, it's no need to take extra time to make this change in another PR at this moment. pyrpkg.rpkgError would be better and we can do that later.

@bowlofeggs Packages for EPEL7 and rawhide are done, and update for EPEL7 has been created in Bodhi.

Thanks so much!

By the way, I just noticed that it does take about 0.6s for bodhi --version to run on my system. I'd rather not let that hold up getting this fedpkg pushed out to EPEL 7. However, if this 0.6s bothers you, I'm happy to add a task to my todo list to come back with a follow up PR sometime soon (probably a month or two) that changes it to do the complex thing we talked about: have it try to use pkg_resources, catch the exception, and fall back to --version. That way it would at least go fast for bodhi 2 users (which is probably most users, or at least eventually will be ☺). Just let me know here and I'll be happy to do it as soon as I can.

Hi @bowlofeggs I'm okay with pushing fedpkg to EPEL7 first, and then improve the bodhi 2's --version.
Metadata
None
No Tags
Changes Summary 1
+27 -3
file changed
src/fedpkg/__init__.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.