diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index d231d52f..eb8070f0 100644 --- a/cli/koji_cli/commands.py +++ b/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): diff --git a/docs/source/writing_koji_code.rst b/docs/source/writing_koji_code.rst index 6aa911b2..41d66925 100644 --- a/docs/source/writing_koji_code.rst +++ b/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 diff --git a/koji/__init__.py b/koji/__init__.py index 4388e8b2..50e15b80 100644 --- a/koji/__init__.py +++ b/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, diff --git a/tests/test_cli/test_set_pkg_arches.py b/tests/test_cli/test_set_pkg_arches.py index 794040d5..0211e441 100644 --- a/tests/test_cli/test_set_pkg_arches.py +++ b/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): diff --git a/tests/test_cli/test_set_pkg_owner.py b/tests/test_cli/test_set_pkg_owner.py index 77bda431..968f6038 100644 --- a/tests/test_cli/test_set_pkg_owner.py +++ b/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): diff --git a/tests/test_cli/test_unblock_pkg.py b/tests/test_cli/test_unblock_pkg.py index 6529c5fe..2b6555cf 100644 --- a/tests/test_cli/test_unblock_pkg.py +++ b/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): diff --git a/tests/test_lib/test_multicall_session.py b/tests/test_lib/test_multicall_session.py new file mode 100644 index 00000000..15b9142d --- /dev/null +++ b/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