#185 Do not check bodhi-client version
Closed 6 years ago by cqi. Opened 6 years ago by cqi.

file modified
+3 -23
@@ -14,11 +14,11 @@ 

  import git

  import re

  import platform

- import subprocess

  

  from . import cli  # noqa

  from .lookaside import FedoraLookasideCache

  from pyrpkg.utils import cached_property

+ from .utils import is_el6_system

  

  

  class Commands(pyrpkg.Commands):
@@ -228,42 +228,22 @@ 

  

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

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

- 

          # 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:

+         if is_el6_system():

              cmd = ['bodhi', '--bodhi-url', bodhi_config['url'],

                     '--new', '--release', self.branch_merge,

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

                     self.user]

-         elif bodhi_major_version < 4:

-             # Version 3 is compatible with 2, it was bumped for server side

-             # reasons.

+         else:

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

                     '--user', self.user]

              if bodhi_config['staging']:

                  cmd.append('--staging')

              cmd.append(self.nvr)

-         else:

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

-             msg = msg.format(bodhi_major_version)

-             raise Exception(msg)

          self._run_command(cmd, shell=True)

  

  

- 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)

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

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

- 

- 

  if __name__ == "__main__":

      from fedpkg.__main__ import main

      main()

file modified
+10
@@ -11,6 +11,7 @@ 

  # the full text of the license.

  

  import re

+ import rpm

  import json

  from datetime import datetime

  
@@ -196,6 +197,15 @@ 

      return bool(re.match(r'^(?:el|epel)\d+$', branch))

  

  

+ def is_el6_system():

+     """Check if current system is EL6

+ 

+     :return: True if current system is EL6. Otherwise False is returned.

+     :rtype: bool

+     """

+     return '.el6' == rpm.expandMacro('%{dist}')

+ 

+ 

  def assert_valid_epel_package(name, branch):

      """

      Determines if the package is allowed to have an EPEL branch. If it can't,

file modified
+4 -22
@@ -43,9 +43,9 @@ 

          self.mock_run_command = self.run_command_patcher.start()

  

          # Let's always use the bodhi 2 command line to test here

-         self.get_bodhi_version_patcher = patch('fedpkg._get_bodhi_version',

-                                                return_value=[2, 11, 0])

-         self.mock_get_bodhi_version = self.get_bodhi_version_patcher.start()

+         self.is_el6_system_patcher = patch('fedpkg.is_el6_system',

+                                            return_value=False)

+         self.mock_is_el6_system = self.is_el6_system_patcher.start()

  

          # Not write clog actually. Instead, file object will be mocked and

          # return fake clog content for tests.
@@ -64,7 +64,7 @@ 

      def tearDown(self):

          self.os_environ_patcher.stop()

          self.clog_patcher.stop()

-         self.get_bodhi_version_patcher.stop()

+         self.is_el6_system_patcher.stop()

          self.run_command_patcher.stop()

          self.nvr_patcher.stop()

          super(TestUpdate, self).tearDown()
@@ -151,28 +151,10 @@ 

      @patch('os.path.isfile', return_value=True)

      @patch('hashlib.new')

      @patch('fedpkg.lookaside.FedoraLookasideCache.hash_file')

-     def test_fail_if_bodhi_version_is_not_supported(

-             self, hash_file, hashlib_new, isfile):

-         # As of writing this test, only supports version v3, v2, and <v2.

-         self.mock_get_bodhi_version.return_value = [4, 1, 2]

-         hashlib_new.return_value.hexdigest.return_value = 'origin hash'

-         hash_file.return_value = 'different hash'

- 

-         cli_cmd = ['fedpkg', '--path', self.cloned_repo_path, 'update']

- 

-         cli = self.get_cli(cli_cmd)

-         six.assertRaisesRegex(

-             self, rpkgError, 'This system has bodhi v4, which is unsupported',

-             self.create_bodhi_update, cli)

- 

-     @patch('os.path.isfile', return_value=True)

-     @patch('hashlib.new')

-     @patch('fedpkg.lookaside.FedoraLookasideCache.hash_file')

      @patch('fedpkg.Commands.user', new_callable=PropertyMock)

      def test_create_update_in_stage_bodhi(

              self, user, hash_file, hashlib_new, isfile):

          user.return_value = 'someone'

-         self.mock_get_bodhi_version.return_value = [2, 8, 1]

          hashlib_new.return_value.hexdigest.return_value = 'origin hash'

          hash_file.return_value = 'different hash'

  

file modified
-14
@@ -9,10 +9,7 @@ 

  # option) any later version.  See http://www.gnu.org/copyleft/gpl.html for

  # the full text of the license.

  

- import unittest

- 

  from pyrpkg.errors import rpkgError

- from fedpkg import _get_bodhi_version

  from utils import CommandTestCase

  from mock import call, patch, Mock, PropertyMock, mock_open

  from six.moves import builtins
@@ -141,17 +138,6 @@ 

          self.assertEqual('someone', self.cmd._user)

  

  

- class GetBodhiVersion(unittest.TestCase):

-     """Test fedpkg._get_bodhi_version"""

- 

-     @patch('subprocess.Popen')

-     def test_get_bodhi_version(self, Popen):

-         Popen.return_value.communicate.return_value = ('1.2.3\n', '')

- 

-         version = _get_bodhi_version()

-         self.assertEqual([1, 2, 3], version)

- 

- 

  class TestLookaside(CommandTestCase):

      """Test Commands.lookasidecache"""

  

file modified
+8
@@ -141,3 +141,11 @@ 

          expected = set(['el6', 'epel7', 'f25', 'f26', 'f27'])

          actual = utils.get_release_branches('http://bodhi.local')

          self.assertEqual(expected, actual)

+ 

+     @patch('fedpkg.utils.rpm.expandMacro')

+     def test_is_el6_system(self, expandMacro):

+         expandMacro.return_value = '.el6'

+         self.assertTrue(utils.is_el6_system())

+ 

+         expandMacro.return_value = '.fc27'

+         self.assertFalse(utils.is_el6_system())

So far, EL6 is still needed to be considered for fedpkg, so
bodhi-client 1.x must be supported as well.

This patch does not check bodhi-client version to select proper version
of bodhi-client. Instead, if current system is EL6, version 1.x is
selected, otherwise bodhi command line will be constructed in the 2.x
format. This avoids fedpkg to be updated and released when each time a
new version of bodhi-client with increased major version is released.

Fixes #171

Signed-off-by: Chenxiong Qi cqi@redhat.com

rebased onto 2d7e345

6 years ago

I think this would work, but it seems more fragile than checking the version of actual executable. What if someone builds their own package with backported version of bodhi client? I don't think this is a common use case, but since we need to keep some check, why not check on the most specific value?.

Pull-Request has been closed by cqi

6 years ago

Make sense to me. Let's see how future bodhi releases impact the check in fedpkg.

(Comment was not added when close PR :)