wireproto: remove iterbatch() from peer interface (API)
authorGregory Szorc <gregory.szorc@gmail.com>
Wed, 11 Apr 2018 16:18:26 -0700
changeset 37633 33a6eee08db2
parent 37632 6c55ce51d6c3
child 37634 0ed11f9368fd
wireproto: remove iterbatch() from peer interface (API) Good riddance. Some tests have been ported to the new API. This probably should have been done in earlier commits. But duplicating the test coverage would have been difficult. It was easier this way. .. api:: The wire protocol peer's ``iterbatch()`` for bulk executing commands has been remove.d Use ``peer.commandexecutor()`` instead. Differential Revision: https://phab.mercurial-scm.org/D3271
mercurial/localrepo.py
mercurial/repository.py
mercurial/wireprotov1peer.py
tests/test-batching.py
tests/test-batching.py.out
tests/test-wireproto.py
--- a/mercurial/localrepo.py	Fri Apr 13 11:08:46 2018 -0700
+++ b/mercurial/localrepo.py	Wed Apr 11 16:18:26 2018 -0700
@@ -66,7 +66,6 @@
     txnutil,
     util,
     vfs as vfsmod,
-    wireprotov1peer,
 )
 from .utils import (
     procutil,
@@ -154,20 +153,6 @@
               'unbundle'}
 legacycaps = moderncaps.union({'changegroupsubset'})
 
-class localiterbatcher(wireprotov1peer.iterbatcher):
-    def __init__(self, local):
-        super(localiterbatcher, self).__init__()
-        self.local = local
-
-    def submit(self):
-        # submit for a local iter batcher is a noop
-        pass
-
-    def results(self):
-        for name, args, opts, resref in self.calls:
-            resref.set(getattr(self.local, name)(*args, **opts))
-            yield resref.value
-
 @zi.implementer(repository.ipeercommandexecutor)
 class localcommandexecutor(object):
     def __init__(self, peer):
@@ -333,9 +318,6 @@
     def commandexecutor(self):
         return localcommandexecutor(self)
 
-    def iterbatch(self):
-        return localiterbatcher(self)
-
     # End of peer interface.
 
 class locallegacypeer(repository.legacypeer, localpeer):
--- a/mercurial/repository.py	Fri Apr 13 11:08:46 2018 -0700
+++ b/mercurial/repository.py	Wed Apr 11 16:18:26 2018 -0700
@@ -284,29 +284,6 @@
 
     All peer instances must conform to this interface.
     """
-    def iterbatch():
-        """Obtain an object to be used for multiple method calls.
-
-        Various operations call several methods on peer instances. If each
-        method call were performed immediately and serially, this would
-        require round trips to remote peers and/or would slow down execution.
-
-        Some peers have the ability to "batch" method calls to avoid costly
-        round trips or to facilitate concurrent execution.
-
-        This method returns an object that can be used to indicate intent to
-        perform batched method calls.
-
-        The returned object is a proxy of this peer. It intercepts calls to
-        batchable methods and queues them instead of performing them
-        immediately. This proxy object has a ``submit`` method that will
-        perform all queued batchable method calls. A ``results()`` method
-        exposes the results of queued/batched method calls. It is a generator
-        of results in the order they were called.
-
-        Not all peers or wire protocol implementations may actually batch method
-        calls. However, they must all support this API.
-        """
 
 class ipeerbaselegacycommands(ipeerbase, ipeerlegacycommands):
     """Unified peer interface that supports legacy commands."""
--- a/mercurial/wireprotov1peer.py	Fri Apr 13 11:08:46 2018 -0700
+++ b/mercurial/wireprotov1peer.py	Wed Apr 11 16:18:26 2018 -0700
@@ -73,97 +73,6 @@
             raise error.RepoError("future is already set")
         self.value = value
 
-class batcher(object):
-    '''base class for batches of commands submittable in a single request
-
-    All methods invoked on instances of this class are simply queued and
-    return a a future for the result. Once you call submit(), all the queued
-    calls are performed and the results set in their respective futures.
-    '''
-    def __init__(self):
-        self.calls = []
-    def __getattr__(self, name):
-        def call(*args, **opts):
-            resref = future()
-            # Please don't invent non-ascii method names, or you will
-            # give core hg a very sad time.
-            self.calls.append((name.encode('ascii'), args, opts, resref,))
-            return resref
-        return call
-    def submit(self):
-        raise NotImplementedError()
-
-class iterbatcher(batcher):
-
-    def submit(self):
-        raise NotImplementedError()
-
-    def results(self):
-        raise NotImplementedError()
-
-class remoteiterbatcher(iterbatcher):
-    def __init__(self, remote):
-        super(remoteiterbatcher, self).__init__()
-        self._remote = remote
-
-    def __getattr__(self, name):
-        # Validate this method is batchable, since submit() only supports
-        # batchable methods.
-        fn = getattr(self._remote, name)
-        if not getattr(fn, 'batchable', None):
-            raise error.ProgrammingError('Attempted to batch a non-batchable '
-                                         'call to %r' % name)
-
-        return super(remoteiterbatcher, self).__getattr__(name)
-
-    def submit(self):
-        """Break the batch request into many patch calls and pipeline them.
-
-        This is mostly valuable over http where request sizes can be
-        limited, but can be used in other places as well.
-        """
-        # 2-tuple of (command, arguments) that represents what will be
-        # sent over the wire.
-        requests = []
-
-        # 4-tuple of (command, final future, @batchable generator, remote
-        # future).
-        results = []
-
-        for command, args, opts, finalfuture in self.calls:
-            mtd = getattr(self._remote, command)
-            batchable = mtd.batchable(mtd.__self__, *args, **opts)
-
-            commandargs, fremote = next(batchable)
-            assert fremote
-            requests.append((command, commandargs))
-            results.append((command, finalfuture, batchable, fremote))
-
-        if requests:
-            self._resultiter = self._remote._submitbatch(requests)
-
-        self._results = results
-
-    def results(self):
-        for command, finalfuture, batchable, remotefuture in self._results:
-            # Get the raw result, set it in the remote future, feed it
-            # back into the @batchable generator so it can be decoded, and
-            # set the result on the final future to this value.
-            remoteresult = next(self._resultiter)
-            remotefuture.set(remoteresult)
-            finalfuture.set(next(batchable))
-
-            # Verify our @batchable generators only emit 2 values.
-            try:
-                next(batchable)
-            except StopIteration:
-                pass
-            else:
-                raise error.ProgrammingError('%s @batchable generator emitted '
-                                             'unexpected value count' % command)
-
-            yield finalfuture.value
-
 def encodebatchcmds(req):
     """Return a ``cmds`` argument value for the ``batch`` command."""
     escapearg = wireprototypes.escapebatcharg
@@ -412,9 +321,6 @@
 
     # Begin of ipeercommands interface.
 
-    def iterbatch(self):
-        return remoteiterbatcher(self)
-
     @batchable
     def lookup(self, key):
         self.requirecap('lookup', _('look up remote revision'))
--- a/tests/test-batching.py	Fri Apr 13 11:08:46 2018 -0700
+++ b/tests/test-batching.py	Wed Apr 11 16:18:26 2018 -0700
@@ -7,10 +7,10 @@
 
 from __future__ import absolute_import, print_function
 
+import contextlib
+
 from mercurial import (
-    error,
     localrepo,
-    util,
     wireprotov1peer,
 
 )
@@ -30,9 +30,14 @@
         return "%s und %s" % (b, a,)
     def greet(self, name=None):
         return "Hello, %s" % name
-    def batchiter(self):
-        '''Support for local batching.'''
-        return localrepo.localiterbatcher(self)
+
+    @contextlib.contextmanager
+    def commandexecutor(self):
+        e = localrepo.localcommandexecutor(self)
+        try:
+            yield e
+        finally:
+            e.close()
 
 # usage of "thing" interface
 def use(it):
@@ -45,52 +50,15 @@
     print(it.bar("Eins", "Zwei"))
 
     # Batched call to a couple of proxied methods.
-    batch = it.batchiter()
-    # The calls return futures to eventually hold results.
-    foo = batch.foo(one="One", two="Two")
-    bar = batch.bar("Eins", "Zwei")
-    bar2 = batch.bar(b="Uno", a="Due")
-
-    # Future shouldn't be set until we submit().
-    assert isinstance(foo, wireprotov1peer.future)
-    assert not util.safehasattr(foo, 'value')
-    assert not util.safehasattr(bar, 'value')
-    batch.submit()
-    # Call results() to obtain results as a generator.
-    results = batch.results()
 
-    # Future results shouldn't be set until we consume a value.
-    assert not util.safehasattr(foo, 'value')
-    foovalue = next(results)
-    assert util.safehasattr(foo, 'value')
-    assert foovalue == foo.value
-    print(foo.value)
-    next(results)
-    print(bar.value)
-    next(results)
-    print(bar2.value)
+    with it.commandexecutor() as e:
+        ffoo = e.callcommand('foo', {'one': 'One', 'two': 'Two'})
+        fbar = e.callcommand('bar', {'b': 'Eins', 'a': 'Zwei'})
+        fbar2 = e.callcommand('bar', {'b': 'Uno', 'a': 'Due'})
 
-    # We should be at the end of the results generator.
-    try:
-        next(results)
-    except StopIteration:
-        print('proper end of results generator')
-    else:
-        print('extra emitted element!')
-
-    # Attempting to call a non-batchable method inside a batch fails.
-    batch = it.batchiter()
-    try:
-        batch.greet(name='John Smith')
-    except error.ProgrammingError as e:
-        print(e)
-
-    # Attempting to call a local method inside a batch fails.
-    batch = it.batchiter()
-    try:
-        batch.hello()
-    except error.ProgrammingError as e:
-        print(e)
+    print(ffoo.result())
+    print(fbar.result())
+    print(fbar2.result())
 
 # local usage
 mylocal = localthing()
@@ -177,8 +145,13 @@
         for r in res.split(';'):
             yield r
 
-    def batchiter(self):
-        return wireprotov1peer.remoteiterbatcher(self)
+    @contextlib.contextmanager
+    def commandexecutor(self):
+        e = wireprotov1peer.peerexecutor(self)
+        try:
+            yield e
+        finally:
+            e.close()
 
     @wireprotov1peer.batchable
     def foo(self, one, two=None):
--- a/tests/test-batching.py.out	Fri Apr 13 11:08:46 2018 -0700
+++ b/tests/test-batching.py.out	Wed Apr 11 16:18:26 2018 -0700
@@ -6,7 +6,6 @@
 One and Two
 Eins und Zwei
 Uno und Due
-proper end of results generator
 
 == Remote
 Ready.
@@ -21,6 +20,3 @@
 One and Two
 Eins und Zwei
 Uno und Due
-proper end of results generator
-Attempted to batch a non-batchable call to 'greet'
-Attempted to batch a non-batchable call to 'hello'
--- a/tests/test-wireproto.py	Fri Apr 13 11:08:46 2018 -0700
+++ b/tests/test-wireproto.py	Wed Apr 11 16:18:26 2018 -0700
@@ -93,7 +93,9 @@
 clt = clientpeer(srv, uimod.ui())
 
 print(clt.greet(b"Foobar"))
-b = clt.iterbatch()
-list(map(b.greet, (b'Fo, =;:<o', b'Bar')))
-b.submit()
-print([r for r in b.results()])
+
+with clt.commandexecutor() as e:
+    fgreet1 = e.callcommand(b'greet', {b'name': b'Fo, =;:<o'})
+    fgreet2 = e.callcommand(b'greet', {b'name': b'Bar'})
+
+print([f.result() for f in (fgreet1, fgreet2)])