PR#957: New multicall interface

Merges #957
https://pagure.io/koji/pull-request/957

cli/koji_cli/commands.py

@@ -6294,9 +6294,9 @@ def handle_set_pkg_arches(goptions, session, args):
     activate_session(session, goptions)
     arches = koji.parse_arches(args[0])
     tag = args[1]
-    for package in args[2:]:
-        #really should implement multicall...
-        session.packageListSetArches(tag,package,arches,force=options.force)
+    with session.multicall(strict=True) as m:
+        for package in args[2:]:
+            m.packageListSetArches(tag,package,arches,force=options.force)
 
 
 def handle_set_pkg_owner(goptions, session, args):
@@ -6312,9 +6312,9 @@ def handle_set_pkg_owner(goptions, session, args):
     activate_session(session, goptions)
     owner = args[0]
     tag = args[1]
-    for package in args[2:]:
-        #really should implement multicall...
-        session.packageListSetOwner(tag,package,owner,force=options.force)
+    with session.multicall(strict=True) as m:
+        for package in args[2:]:
+            m.packageListSetOwner(tag,package,owner,force=options.force)
 
 
 def handle_set_pkg_owner_global(goptions, session, args):
@@ -6638,9 +6638,9 @@ def handle_unblock_pkg(goptions, session, args):
         assert False  # pragma: no cover
     activate_session(session, goptions)
     tag = args[0]
-    for package in args[1:]:
-        #really should implement multicall...
-        session.packageListUnblock(tag,package)
+    with session.multicall(strict=True) as m:
+        for package in args[1:]:
+            m.packageListUnblock(tag,package)
 
 
 def anon_handle_download_build(options, session, args):

docs/source/writing_koji_code.rst

@@ -174,7 +174,7 @@ can fix this behavior in ``koji/__init__.py`` in the \_taskLabel
 function. Here you can define the string(s) to display when Koji
 receives status on a task. That is the return value.
 
-Using multiCall
+Using multicall
 ~~~~~~~~~~~~~~~
 
 Koji supports a multicall feature where many calls are passed to the
@@ -185,6 +185,73 @@ The ``ClientSession`` class provides support for this and there are several
 examples in the existing client code. Some examples in the cli include:
 ``edit-host``, ``add-pkg``, ``disable-host``, and ``list-hosts``.
 
+There are two ways to use multicall.
+The original modal method works within the ``ClientSession`` object and
+prevents making other normal calls until the multicall is completed.
+The newer method uses a separate ``MultiCallSession`` object and is much
+more flexible.
+
+**Using MultiCallSession**
+
+Note: this feature was added in Koji version 1.18.
+
+A ``MultiCallSession`` object is used to track an individual multicall attached
+to a session.
+To create one, you can simply call your session's ``multicall`` method.
+Once created, the object can be used like a session, but calls are stored
+rather than sent immediately.
+The stored calls are executed by calling the ``call_all()`` method.
+
+::
+
+    m = session.multicall()
+    for task_id in mylist:
+        m.cancelTask(task_id)
+    m.call_all()
+
+This object can also be used as a context manager, so the following is
+equivalent:
+
+::
+
+    with session.multicall() as m:
+        for task_id in mylist:
+            m.cancelTask(task_id)
+
+Method calls to a ``MultiCallSession`` object return a ``VirtualCall`` object
+that stands in for the result.
+Once the multicall is executed, the result of each call can be accessed via
+the ``result`` property of the ``VirtualCall`` object.
+Accessing the ``result`` property before the call is executed will result in
+an error.
+
+::
+
+    with session.multicall() as m:
+        tags = [m.getTag(tag_id) for tag_id in mylist]
+    for tag in tags:
+        print(tag.result['name'])
+
+There are two parameters affecting the behavior of the multicall.
+If the ``strict`` parameter is set to True, the multicall will raise the first
+error it encounters, if any.
+If the ``batch`` parameter is set to a number greater than zero, the multicall
+will spread the calls across multiple multicall batches of at most that number.
+These parameters may be passed when the ``MultiCallSession`` is initialized,
+or they may be passed to the ``call_all`` method.
+
+::
+
+    with session.multicall(strict=True, batch=500):
+        builds = [m.getBuild(build_id) for build_id in mylist]
+
+
+**Using ClientSession.multiCall**
+
+Note: this approach is still supported, but we highly recommend using
+``MultiCallSession`` as described above, unless you need to support Koji
+versions prior to 1.18.
+
 To use the feature, you first set the ``multicall`` attribute of the session
 to ``True``. Once this is done, the session will not immediately process
 further calls but will instead store their parameters for later. To tell the

koji/__init__.py

@@ -2132,13 +2132,27 @@ class ClientSession(object):
         self.opts = opts
         self.authtype = None
         self.setSession(sinfo)
-        self.multicall = False
+        self._multicall = MultiCallHack(self)
         self._calls = []
         self.logger = logging.getLogger('koji')
         self.rsession = None
         self.new_session()
         self.opts.setdefault('timeout', DEFAULT_REQUEST_TIMEOUT)
 
+    @property
+    def multicall(self):
+        """The multicall property acts as a settable boolean or a callable
+
+        This setup allows preserving the original multicall interface
+        alongside the new one without adding yet another similar sounding
+        attribute to the session (we already have both multicall and
+        multiCall).
+        """
+        return self._multicall
+
+    @multicall.setter
+    def multicall(self, value):
+        self._multicall.value = value
 
     def new_session(self):
         self.logger.debug("Opening new requests session")
@@ -2858,6 +2872,143 @@ class ClientSession(object):
         return base64.b64decode(result)
 
 
+class MultiCallHack(object):
+    """Workaround of a terribly overloaded namespace
+
+    This allows session.multicall to act as a boolean value or a callable
+    """
+
+    def __init__(self, session):
+        self.value = False
+        self.session = session
+
+    def __nonzero__(self):
+        return self.value
+
+    def __bool__(self):
+        return self.value
+
+    def __call__(self, **kw):
+        return MultiCallSession(self.session, **kw)
+
+
+class MultiCallNotReady(Exception):
+    """Raised when a multicall result is accessed before the multicall"""
+    pass
+
+
+class VirtualCall(object):
+    """Represents a call within a multicall"""
+
+    def __init__(self, method, args, kwargs):
+        self.method = method
+        self.args = args
+        self.kwargs = kwargs
+        self._result = MultiCallInProgress()
+
+    def format(self):
+        '''return the call in the format needed for multiCall'''
+        return {'methodName': self.method,
+                'params': encode_args(*self.args, **self.kwargs)}
+
+    @property
+    def result(self):
+        result = self._result
+        if isinstance(result, MultiCallInProgress):
+            raise MultiCallNotReady()
+        if isinstance(result, dict):
+            fault = Fault(result['faultCode'], result['faultString'])
+            err = convertFault(fault)
+            raise err
+        # otherwise should be a singleton
+        return result[0]
+
+
+class MultiCallSession(object):
+
+    """Manages a single multicall, acts like a session"""
+
+    def __init__(self, session, strict=False, batch=None):
+        self._session = session
+        self._strict = strict
+        self._batch = batch
+        self._calls = []
+
+    def __getattr__(self, name):
+        return VirtualMethod(self._callMethod, name)
+
+    def _callMethod(self, name, args, kwargs=None, retry=True):
+        """Add a new call to the multicall"""
+
+        if kwargs is None:
+            kwargs = {}
+        ret = VirtualCall(name, args, kwargs)
+        self._calls.append(ret)
+        return ret
+
+    def callMethod(self, name, *args, **opts):
+        """compatibility wrapper for _callMethod"""
+        return self._callMethod(name, args, opts)
+
+    def call_all(self, strict=None, batch=None):
+        """Perform all calls in one or more multiCall batches
+
+        Returns a list of results for each call. For successful calls, the
+        entry will be a singleton list. For calls that raised a fault, the
+        entry will be a dictionary with keys "faultCode", "faultString",
+        and "traceback".
+        """
+
+        if strict is None:
+            strict = self._strict
+        if batch is None:
+            batch = self._batch
+
+        if len(self._calls) == 0:
+            return []
+
+        calls = self._calls
+        self._calls = []
+        if batch:
+            batches = [calls[i:i+batch] for i in range(0, len(calls), batch)]
+        else:
+            batches = [calls]
+        results = []
+        for calls in batches:
+            args = ([c.format() for c in calls],)
+            _results = self._session._callMethod('multiCall', args, {})
+            for call, result in zip(calls, _results):
+                call._result = result
+            results.extend(_results)
+        if strict:
+            # check for faults and raise first one
+            for entry in results:
+                if isinstance(entry, dict):
+                    fault = Fault(entry['faultCode'], entry['faultString'])
+                    err = convertFault(fault)
+                    raise err
+        return results
+
+    # alias for compatibility with ClientSession
+    multiCall = call_all
+
+    # more backwards compat
+    # multicall returns True but cannot be set
+    @property
+    def multicall():
+        return True
+
+    # implement a context manager
+    def __enter__(self):
+        return self
+
+    def __exit__(self, _type, value, traceback):
+        if _type is None:
+            self.call_all()
+        # don't eat exceptions
+        return False
+
+
 class DBHandler(logging.Handler):
     """
     A handler class which writes logging records, appropriately formatted,

tests/test_cli/test_set_pkg_arches.py

@@ -47,10 +47,13 @@ class TestSetPkgArches(utils.CliTestCase):
         activate_session_mock.assert_not_called()
 
         # Case 2. run set arch to x86_64
+        multicall = mock.MagicMock()
+        multicall.__enter__.return_value = multicall
+        session.multicall.return_value = multicall
         calls = [mock.call('tag', pkg, 'x86_64', force=True) for pkg in arguments[3:]]
         handle_set_pkg_arches(options, session, arguments)
         activate_session_mock.assert_called_with(session, options)
-        session.packageListSetArches.assert_has_calls(calls)
+        multicall.packageListSetArches.assert_has_calls(calls)
         self.assert_console_message(stdout, '')
 
     def test_handle_set_pkg_arches_help(self):

tests/test_cli/test_set_pkg_owner.py

@@ -47,10 +47,13 @@ class TestSetPkgOwner(utils.CliTestCase):
         activate_session_mock.assert_not_called()
 
         # Case 2. run set owner
+        multicall = mock.MagicMock()
+        multicall.__enter__.return_value = multicall
+        session.multicall.return_value = multicall
         calls = [mock.call('tag', pkg, 'owner', force=True) for pkg in arguments[3:]]
         handle_set_pkg_owner(options, session, arguments)
         activate_session_mock.assert_called_with(session, options)
-        session.packageListSetOwner.assert_has_calls(calls)
+        multicall.packageListSetOwner.assert_has_calls(calls)
         self.assert_console_message(stdout, '')
 
     def test_handle_set_pkg_owner_help(self):

tests/test_cli/test_unblock_pkg.py

@@ -48,10 +48,13 @@ class TestUnblockPkg(utils.CliTestCase):
         activate_session_mock.assert_not_called()
 
         # Case 2. run unlock
+        multicall = mock.MagicMock()
+        multicall.__enter__.return_value = multicall
+        session.multicall.return_value = multicall
         calls = [mock.call('tag', pkg) for pkg in arguments[1:]]
         handle_unblock_pkg(options, session, arguments)
         activate_session_mock.assert_called_with(session, options)
-        session.packageListUnblock.assert_has_calls(calls)
+        multicall.packageListUnblock.assert_has_calls(calls)
         self.assert_console_message(stdout, '')
 
     def test_handle_unblock_pkg_help(self):

tests/test_lib/test_multicall_session.py

@@ -0,0 +1,55 @@
+import mock
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+import koji
+
+
+class TestNewMultiCall(unittest.TestCase):
+
+    def setUp(self):
+        self._callMethod = mock.patch('koji.ClientSession._callMethod').start()
+        self.session = koji.ClientSession('FAKE_URL')
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_basic_multicall(self):
+        with self.session.multicall() as m:
+            ret = {}
+            for i in range(10):
+                ret[i] = m.echo(i)
+        self._callMethod.assert_called_once()
+        self.assertEqual(self._callMethod.call_args[0][0], 'multiCall')
+        self.assertEqual(self._callMethod.call_args[0][2], {})
+        _calls = self._callMethod.call_args[0][1]
+        if not isinstance(_calls, tuple) or len(_calls) != 1:
+            raise Exception('multiCall args not wrapped in singleton')
+        calls = _calls[0]
+        for i in range(10):
+            self.assertEqual(calls[i]['methodName'], "echo")
+            self.assertEqual(calls[i]['params'], (i,))
+
+    def test_batch_multicall(self):
+        with self.session.multicall(batch=10) as m:
+            ret = {}
+            for i in range(42):
+                ret[i] = m.echo(i)
+
+        # should be 5 batches
+        self.assertEqual(self._callMethod.call_count, 5)
+        i = 0
+        for args, kwargs in self._callMethod.call_args_list:
+            self.assertEqual(kwargs, {})
+            self.assertEqual(args[0], 'multiCall')
+            self.assertEqual(args[2], {})
+            _calls = args[1]
+            if not isinstance(_calls, tuple) or len(_calls) != 1:
+                raise Exception('multiCall args not wrapped in singleton')
+            calls = _calls[0]
+            for call in calls:
+                self.assertEqual(call['methodName'], "echo")
+                self.assertEqual(call['params'], (i,))
+                i += 1