PR#226: Drop support of bodhi-client 0.9 - fedpkg

		`@@ -18,10 +18,6 @@`
		`kerberos_realms = FEDORAPROJECT.ORG`

		`[fedpkg.bodhi]`
		`- # This is for the bodhi-client 1.x. that accepts --bodhi-dir option to switch`
		`- # to different instances including the production and stage.`
		`- url = https://bodhi.fedoraproject.org/`
		`-`
		`# This is for the bodhi-client 2.x, that do not require an option to switch to`
		`# different instance. Instead, --staging is available to switch to the stage`
		`# bodhi, and production is used without providing --staging.`

fedpkg/__init__.py

file modified

+5 -37

		`@@ -14,7 +14,6 @@`
		`import git`
		`import re`
		`import platform`
		`- import subprocess`

		`from . import cli # noqa`
		`from .lookaside import FedoraLookasideCache`
		`@@ -228,45 +227,14 @@`

		`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_major_version()`
		`- if bodhi_major_version < 2:`
		`- 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.`
		`- 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)`
		`+ cmd = ['bodhi', 'updates', 'new', '--file', 'bodhi.template',`
		`+ '--user', self.user]`
		`+ if bodhi_config['staging']:`
		`+ cmd.append('--staging')`
		`+ cmd.append(self.nvr)`
		`self._run_command(cmd, shell=True)`


		`- def _get_bodhi_major_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.`
		`- """`
		`- bodhi = subprocess.Popen(['bodhi', '--version'],`
		`- stdout=subprocess.PIPE,`
		`- universal_newlines=True)`
		`- version = bodhi.communicate()[0].strip()`
		`- major, _ = version.split('.', 1)`
		`- return int(major)`
		`-`
		`-`
		`if __name__ == "__main__":`
		`from fedpkg.__main__ import main`
		`main()`

fedpkg/cli.py

file modified

+14 -1

		`@@ -18,6 +18,7 @@`
		`import os`
		`import re`
		`import json`
		`+ import pkg_resources`
		`import six`
		`import textwrap`

		`@@ -34,6 +35,17 @@`
		`RELEASE_BRANCH_REGEX = r'^(f\d+\|el\d+\|epel\d+)$'`


		`+ def check_bodhi_version():`
		`+ try:`
		`+ dist = pkg_resources.get_distribution('bodhi_client')`
		`+ except pkg_resources.DistributionNotFound:`
		`+ raise rpkgError('bodhi-client < 2.0 is not supported.')`
		`+ major = int(dist.version.split('.', 1))`
		`+ if major >= 4:`
		`+ raise rpkgError(`
		`+ 'This system has bodhi v{0}, which is unsupported.'.format(major))`
		`+`
		`+`
		`class fedpkgClient(cliClient):`
		`def __init__(self, config, name=None):`
		`self.DEFAULT_CLI_NAME = 'fedpkg'`
		`@@ -243,10 +255,11 @@`
		`return lines[0], "\n".join(log)`

		`def update(self):`
		`+ check_bodhi_version()`
		`+`
		`try:`
		`section = '%s.bodhi' % self.name`
		`bodhi_config = {`
		`- 'url': self.config.get(section, 'url'),`
		`'staging': self.config.getboolean(section, 'staging'),`
		`}`
		`except (ValueError, NoOptionError, NoSectionError) as e:`

test/fedpkg-stage.conf

file modified

-1

		`@@ -11,7 +11,6 @@`
		`kerberos_realms = STG.FEDORAPROJECT.ORG`

		`[fedpkg-stage.bodhi]`
		`- url = https://bodhi.stg.example.com/`
		`staging = True`

		`[fedpkg-stage.bugzilla]`

test/fedpkg-test.conf

file modified

-1

		`@@ -11,7 +11,6 @@`
		`kerberos_realms = FEDORAPROJECT.ORG`

		`[fedpkg.bodhi]`
		`- url = https://bodhi.dummy.example.com/`
		`staging = False`

		`[fedpkg.bugzilla]`

test/test_cli.py

file modified

+19 -23

		`@@ -14,6 +14,9 @@`
		`import os`
		`import sys`
		`import json`
		`+ import pkg_resources`
		`+ import unittest`
		`+`
		`from datetime import datetime, timedelta`
		`from tempfile import mkdtemp`
		`from os import rmdir`
		`@@ -26,6 +29,7 @@`
		`from pyrpkg.errors import rpkgError`
		`from utils import CliTestCase`
		`from fedpkg.bugzilla import BugzillaClient`
		`+ from fedpkg.cli import check_bodhi_version`

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

		`@@ -45,10 +49,8 @@`
		`self.mock_run_command = self.run_command_patcher.start()`

		`# Let's always use the bodhi 2 command line to test here`
		`- self.get_bodhi_major_version_patcher = patch(`
		`- 'fedpkg._get_bodhi_major_version', return_value=2)`
		`- self.mock_get_bodhi_major_version = \`
		`- self.get_bodhi_major_version_patcher.start()`
		`+ self.check_bodhi_version_patcher = patch('fedpkg.cli.check_bodhi_version')`
		`+ self.mock_check_bodhi_version = self.check_bodhi_version_patcher.start()`

		`# Not write clog actually. Instead, file object will be mocked and`
		`# return fake clog content for tests.`
		`@@ -74,7 +76,7 @@`
		`os.unlink(os.path.join(self.cloned_repo_path, 'clog'))`
		`self.os_environ_patcher.stop()`
		`self.clog_patcher.stop()`
		`- self.get_bodhi_major_version_patcher.stop()`
		`+ self.check_bodhi_version_patcher.stop()`
		`self.run_command_patcher.stop()`
		`self.nvr_patcher.stop()`
		`super(TestUpdate, self).tearDown()`
		`@@ -168,28 +170,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_major_version.return_value = 4`
		`- 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.assert_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_major_version.return_value = 2`
		`hashlib_new.return_value.hexdigest.return_value = 'origin hash'`
		`hash_file.return_value = 'different hash'`

		`@@ -1126,3 +1110,15 @@`
		`assert False, 'rpkgError not raised'`
		`except rpkgError as error:`
		`self.assertEqual(str(error), expected_error)`
		`+`
		`+`
		`+ class TestCheckBodhiVersion(unittest.TestCase):`
		`+ """Test check_bodhi_version"""`
		`+`
		`+ @patch('pkg_resources.get_distribution')`
		`+ def test_no_2_x_version_installed(self, get_distribution):`
		`+ get_distribution.side_effect = pkg_resources.DistributionNotFound`
		`+`
		`+ six.assertRaisesRegex(`
		`+ self, rpkgError, r'bodhi-client < 2\.0 is not supported\.',`
		`+ check_bodhi_version)`

test/test_commands.py

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_major_version`
		`from utils import CommandTestCase`
		`from mock import call, patch, Mock, PropertyMock, mock_open`
		`from six.moves import builtins`
		`@@ -138,17 +135,6 @@`
		`self.assertEqual(getuser.return_value, self.cmd._user)`


		`- class GetBodhiVersion(unittest.TestCase):`
		`- """Test fedpkg._get_bodhi_major_version"""`
		`-`
		`- @patch('subprocess.Popen')`
		`- def test_get_bodhi_version(self, Popen):`
		`- Popen.return_value.communicate.return_value = ('1.2.b0\n', '')`
		`-`
		`- version = _get_bodhi_major_version()`
		`- self.assertEqual(1, version)`
		`-`
		`-`
		`class TestLookaside(CommandTestCase):`
		`"""Test Commands.lookasidecache"""`

cqi commented 7 years ago

Fixes #223

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

lsedlar commented on line 63 of fedpkg/__init__.py 7 years ago

Is this correct? Wouldn't it report all existing versions as unsupported? Even Rawhide has only 3.8.0.

cqi commented on line 63 of fedpkg/__init__.py 7 years ago

My silly mistake. Sorry.

rebased onto c2c9791

7 years ago

cqi commented 7 years ago

pretty please pagure-ci rebuild

cqi commented 7 years ago

@lsedlar Fixed. PTAL.

lsedlar commented 7 years ago

The change looks good to me now.
I'm wondering if it would make sense to check the version sooner. Current code will let user fill in the template, then check the version, report an error and throw the template away. That's pretty evil. Though that should probably be a different PR.

cqi commented 7 years ago

I'm wondering if it would make sense to check the version sooner. Current code will let user fill in the template, then check the version, report an error and throw the template away. That's pretty evil. Though that should probably be a different PR.

Good Point! I'll make a PR for this. I also think to use bodhi client binding instead of the bodhi CLI to rewrite the update.

1 new commit added

Check bodhi version earlier

7 years ago

1 new commit added

Also remove bodhi url from config

7 years ago

cqi commented 7 years ago

pretty please pagure-ci rebuild

lsedlar commented 7 years ago

Looks good to me. :thumbsup:

3 new commits added

Also remove bodhi url from config
Check bodhi version earlier
Drop support of bodhi-client 0.9

7 years ago

cqi commented 7 years ago

@lsedlar There is a minor update to the last commit. That is to restore to pass bodhi config in a dict instead of passing single staging argument. When I was writing code for new override command, I found I'm repeating self.config.getboolean(section, 'staging'), at least three times. This minor change will be helpful to avoid that. Just a single call to get_bodhi_config simply.