From 72164d48f56f91e14b9503d9dbd0f08ad3ada149 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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

