From 72164d48f56f91e14b9503d9dbd0f08ad3ada149 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Nov 06 2023 13:18:50 +0000 Subject: [PATCH 1/4] Don't try to resolve server version for old hubs Related: https://pagure.io/koji/issue/3890 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index b954cff..143d852 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -324,9 +324,8 @@ def handle_add_channel(goptions, session, args): if 'channel %s already exists' % channel_name in msg: error("channel %s already exists" % channel_name) elif 'Invalid method:' in msg: - version = session.getKojiVersion() error("addChannel is available on hub from Koji 1.26 version, your version is %s" % - version) + session.hub_version_str) else: error(msg) print("%s added: id %d" % (args[0], channel_id)) @@ -356,9 +355,8 @@ def handle_edit_channel(goptions, session, args): except koji.GenericError as ex: msg = str(ex) if 'Invalid method:' in msg: - version = session.getKojiVersion() error("editChannel is available on hub from Koji 1.26 version, your version is %s" % - version) + session.hub_version_str) else: warn(msg) if not result: @@ -6213,14 +6211,6 @@ def handle_cancel(goptions, session, args): if len(args) == 0: parser.error("You must specify at least one task id or build") activate_session(session, goptions) - older_hub = False - try: - hub_version = session.getKojiVersion() - v = tuple([int(x) for x in hub_version.split('.')]) - if v < (1, 33, 0): - older_hub = True - except koji.GenericError: - older_hub = True tlist = [] blist = [] for arg in args: @@ -6247,7 +6237,7 @@ def handle_cancel(goptions, session, args): for task_id in tlist: results.append(remote_fn(task_id, **opts)) for build in blist: - if not older_hub: + if session.hub_version >= (1, 33, 0): results.append(m.cancelBuild(build, strict=True)) else: results.append(m.cancelBuild(build)) diff --git a/koji/__init__.py b/koji/__init__.py index 4d0e7b2..6a9d81d 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2617,6 +2617,27 @@ class ClientSession(object): self.opts.setdefault('timeout', DEFAULT_REQUEST_TIMEOUT) self.exclusive = False self.auth_method = auth_method + self.__hub_version = None + + @property + def hub_version(self): + """Cached version of getKojiVersion call for use in version checks""" + # If any call was made before, it should be populated by Koji-Version header + # for hub >= 1.35 + if self.__hub_version is None: + try: + # no call was made yet AND 1.22.0 < hub_version < 1.35 + self.__hub_version = self.getKojiVersion() + except GenericError: + # hub is older than 1.23, return latest version without the getKojiVersion + self.__hub_version = '1.22.0' + return tuple([int(x) for x in self.__hub_version.split('.')]) + + @property + def hub_version_str(self): + # fill cache + self.hub_version + return self.__hub_version @property def multicall(self): @@ -3048,6 +3069,7 @@ class ClientSession(object): warnings.simplefilter("ignore") r = self.rsession.post(handler, **callopts) r.raise_for_status() + self.__hub_version = r.headers.get('Koji-Version') try: ret = self._read_xmlrpc_response(r) finally: diff --git a/kojihub/kojixmlrpc.py b/kojihub/kojixmlrpc.py index 64ff8bc..87ddc9e 100644 --- a/kojihub/kojixmlrpc.py +++ b/kojihub/kojixmlrpc.py @@ -44,6 +44,12 @@ from . import db from . import scheduler +# HTTP headers included in every request +GLOBAL_HEADERS = [ + ('Koji-Version', koji.__version__), +] + + class Marshaller(ExtendedMarshaller): dispatch = ExtendedMarshaller.dispatch.copy() @@ -389,7 +395,7 @@ def offline_reply(start_response, msg=None): else: faultString = msg response = dumps(Fault(faultCode, faultString)).encode() - headers = [ + headers = GLOBAL_HEADERS + [ ('Content-Length', str(len(response))), ('Content-Type', "text/xml"), ] @@ -399,7 +405,7 @@ def offline_reply(start_response, msg=None): def error_reply(start_response, status, response, extra_headers=None): response = response.encode() - headers = [ + headers = GLOBAL_HEADERS + [ ('Content-Length', str(len(response))), ('Content-Type', "text/plain"), ] @@ -811,7 +817,7 @@ def application(environ, start_response): except RequestTimeout as e: return error_reply(start_response, '408 Request Timeout', str(e) + '\n') response = response.encode() - headers = [ + headers = GLOBAL_HEADERS + [ ('Content-Length', str(len(response))), ('Content-Type', "text/xml"), ] diff --git a/tests/test_cli/test_add_channel.py b/tests/test_cli/test_add_channel.py index dc9d991..99f1ea1 100644 --- a/tests/test_cli/test_add_channel.py +++ b/tests/test_cli/test_add_channel.py @@ -60,7 +60,7 @@ class TestAddChannel(utils.CliTestCase): expected_api = 'Invalid method: addChannel' expected = 'addChannel is available on hub from Koji 1.26 version, your version ' \ 'is 1.25.1\n' - self.session.getKojiVersion.return_value = '1.25.1' + self.session.hub_version_str = '1.25.1' self.session.addChannel.side_effect = koji.GenericError(expected_api) arguments = ['--description', self.description, self.channel_name] diff --git a/tests/test_cli/test_cancel.py b/tests/test_cli/test_cancel.py index 16ec5d3..7a8a1c6 100644 --- a/tests/test_cli/test_cancel.py +++ b/tests/test_cli/test_cancel.py @@ -32,7 +32,8 @@ class TestCancel(utils.CliTestCase): %s: error: {message} """ % (self.progname, self.progname) - self.session.getKojiVersion.return_value = '1.33.0' + self.session.hub_version = (1, 33, 0) + self.session.hub_version_str = '1.33.0' def test_anon_cancel(self): args = ['123'] @@ -154,7 +155,7 @@ No such build: '%s' self.session.cancelBuild.assert_called_once_with(args[1], strict=True) def test_non_exist_build_and_task_older_hub(self): - self.session.getKojiVersion.return_value = '1.32.0' + self.session.hub_version = (1, 32, 0) args = ['11111', 'nvr-1-30.1'] expected_warn = """No such task: %s """ % (args[0]) diff --git a/tests/test_cli/test_edit_channel.py b/tests/test_cli/test_edit_channel.py index cbd3fb1..f4c5f05 100644 --- a/tests/test_cli/test_edit_channel.py +++ b/tests/test_cli/test_edit_channel.py @@ -78,7 +78,7 @@ Options: expected_api = 'Invalid method: editChannel' expected = 'editChannel is available on hub from Koji 1.26 version, your version ' \ 'is 1.25.1\n' - self.session.getKojiVersion.return_value = '1.25.1' + self.session.hub_version_str = '1.25.1' self.session.editChannel.side_effect = koji.GenericError(expected_api) self.assert_system_exit( @@ -94,7 +94,6 @@ Options: self.session.editChannel.assert_called_once_with(self.channel_old, name=self.channel_new, description=self.description) self.session.getChannel.assert_called_once_with(self.channel_old) - self.session.getKojiVersion.assert_called_once_with() def test_handle_edit_channel_non_exist_channel(self): expected = 'No such channel: %s\n' % self.channel_old diff --git a/tests/test_lib/test_client_session.py b/tests/test_lib/test_client_session.py index 3f42341..35f2405 100644 --- a/tests/test_lib/test_client_session.py +++ b/tests/test_lib/test_client_session.py @@ -30,6 +30,36 @@ class TestClientSession(unittest.TestCase): my_rsession.close.assert_called() self.assertNotEqual(ksession.rsession, my_rsession) + @mock.patch('requests.Session') + def test_hub_version_old(self, rsession): + ksession = koji.ClientSession('http://koji.example.com/kojihub') + ksession.getKojiVersion = mock.MagicMock() + ksession.getKojiVersion.side_effect = koji.GenericError + self.assertEqual(ksession.hub_version, (1, 22, 0)) + ksession.getKojiVersion.assert_called_once() + + @mock.patch('requests.Session') + def test_hub_version_interim(self, rsession): + ksession = koji.ClientSession('http://koji.example.com/kojihub') + ksession.getKojiVersion = mock.MagicMock() + ksession.getKojiVersion.return_value = '1.23.1' + self.assertEqual(ksession.hub_version, (1, 23, 1)) + ksession.getKojiVersion.assert_called_once() + + def test_hub_version_str_interim(self): + ksession = koji.ClientSession('http://koji.example.com/kojihub') + ksession.getKojiVersion = mock.MagicMock() + ksession.getKojiVersion.return_value = '1.23.1' + self.assertEqual(ksession.hub_version_str, '1.23.1') + + def test_hub_version_new(self): + ksession = koji.ClientSession('http://koji.example.com/kojihub') + ksession.getKojiVersion = mock.MagicMock() + # would be filled by random call + ksession._ClientSession__hub_version = '1.35.0' + self.assertEqual(ksession.hub_version, (1, 35, 0)) + ksession.getKojiVersion.assert_not_called() + class TestFastUpload(unittest.TestCase): From b5d6be932e15058b7b74197e763a5963da43edf7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 07 2023 13:45:31 +0000 Subject: [PATCH 2/4] move version fetching to hub_version_str property --- diff --git a/koji/__init__.py b/koji/__init__.py index 6a9d81d..e2ebdd7 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2621,22 +2621,24 @@ class ClientSession(object): @property def hub_version(self): - """Cached version of getKojiVersion call for use in version checks""" + """Return the hub version as a tuple of ints""" + return tuple([int(x) for x in self.hub_version_str.split('.')]) + + @property + def hub_version_str(self): + """Return the hub version as string""" # If any call was made before, it should be populated by Koji-Version header # for hub >= 1.35 if self.__hub_version is None: + # no call was made yet OR hub_version < 1.35 try: - # no call was made yet AND 1.22.0 < hub_version < 1.35 self.__hub_version = self.getKojiVersion() - except GenericError: - # hub is older than 1.23, return latest version without the getKojiVersion - self.__hub_version = '1.22.0' - return tuple([int(x) for x in self.__hub_version.split('.')]) - - @property - def hub_version_str(self): - # fill cache - self.hub_version + except GenericError as e: + if 'Invalid method' in str(e): + # hub is older than 1.23, return latest version without the getKojiVersion + self.__hub_version = '1.22.0' + else: + raise return self.__hub_version @property diff --git a/tests/test_lib/test_client_session.py b/tests/test_lib/test_client_session.py index 35f2405..fb3de76 100644 --- a/tests/test_lib/test_client_session.py +++ b/tests/test_lib/test_client_session.py @@ -34,7 +34,7 @@ class TestClientSession(unittest.TestCase): def test_hub_version_old(self, rsession): ksession = koji.ClientSession('http://koji.example.com/kojihub') ksession.getKojiVersion = mock.MagicMock() - ksession.getKojiVersion.side_effect = koji.GenericError + ksession.getKojiVersion.side_effect = koji.GenericError('Invalid method: getKojiVersion') self.assertEqual(ksession.hub_version, (1, 22, 0)) ksession.getKojiVersion.assert_called_once() From 4e934d9c21a8437b1caa3f851cada61f7b1bf361 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 09 2023 12:34:23 +0000 Subject: [PATCH 3/4] avoid clearing cached hub version --- diff --git a/koji/__init__.py b/koji/__init__.py index e2ebdd7..60c1c57 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -3071,7 +3071,9 @@ class ClientSession(object): warnings.simplefilter("ignore") r = self.rsession.post(handler, **callopts) r.raise_for_status() - self.__hub_version = r.headers.get('Koji-Version') + hub_version = r.headers.get('Koji-Version') + if hub_version: + self.__hub_version = hub_version try: ret = self._read_xmlrpc_response(r) finally: From f2e3fd2387bb54d051e3801733c12d0ff8760b06 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 09 2023 12:34:23 +0000 Subject: [PATCH 4/4] show hub version in koji hello --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 143d852..0f3f55d 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -7483,7 +7483,7 @@ def handle_moshimoshi(options, session, args): u = {'name': 'anonymous user'} print("%s, %s!" % (_printable_unicode(random.choice(greetings)), u["name"])) print("") - print("You are using the hub at %s" % session.baseurl) + print("You are using the hub at %s (Koji %s)" % (session.baseurl, session.hub_version_str)) authtype = u.get('authtype', getattr(session, 'authtype', None)) if authtype == koji.AUTHTYPES['NORMAL']: print("Authenticated via password") diff --git a/koji/__init__.py b/koji/__init__.py index 60c1c57..470f34e 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2635,7 +2635,8 @@ class ClientSession(object): self.__hub_version = self.getKojiVersion() except GenericError as e: if 'Invalid method' in str(e): - # hub is older than 1.23, return latest version without the getKojiVersion + # use latest version without the getKojiVersion handler + self.logger.debug("hub is older than 1.23, assuming 1.22.0") self.__hub_version = '1.22.0' else: raise diff --git a/tests/test_cli/test_hello.py b/tests/test_cli/test_hello.py index 4571289..6e35638 100644 --- a/tests/test_cli/test_hello.py +++ b/tests/test_cli/test_hello.py @@ -50,6 +50,8 @@ class TestHello(utils.CliTestCase): # Mock out the xmlrpc server session.getLoggedInUser.return_value = None session.krb_principal = user['krb_principal'] + mock_hub_version = '1.35.0' + session.hub_version_str = mock_hub_version print_unicode_mock.return_value = "Hello" self.assert_system_exit( @@ -63,7 +65,7 @@ class TestHello(utils.CliTestCase): # annonymous user message = "Not authenticated\n" + "Hello, anonymous user!" - hubinfo = "You are using the hub at %s" % self.huburl + hubinfo = "You are using the hub at %s (Koji %s)" % (self.huburl, mock_hub_version) handle_moshimoshi(self.options, session, []) self.assert_console_message(stdout, "{0}\n\n{1}\n".format(message, hubinfo)) self.activate_session_mock.assert_called_once_with(session, self.options) @@ -79,7 +81,7 @@ class TestHello(utils.CliTestCase): user['krb_principal'], koji.AUTHTYPES['SSL']: 'Authenticated via client certificate %s' % cert } - hubinfo = "You are using the hub at %s" % self.huburl + # same hubinfo session.getLoggedInUser.return_value = user message = "Hello, %s!" % self.progname self.options.cert = cert