D3271: wireproto: remove iterbatch() from peer interface (API)
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Wed Apr 11 23:43:44 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3271
AFFECTED FILES
mercurial/localrepo.py
mercurial/repository.py
mercurial/wireprotov1peer.py
tests/test-batching.py
tests/test-batching.py.out
tests/test-wireproto.py
CHANGE DETAILS
diff --git a/tests/test-wireproto.py b/tests/test-wireproto.py
--- a/tests/test-wireproto.py
+++ b/tests/test-wireproto.py
@@ -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)])
diff --git a/tests/test-batching.py.out b/tests/test-batching.py.out
--- a/tests/test-batching.py.out
+++ b/tests/test-batching.py.out
@@ -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'
diff --git a/tests/test-batching.py b/tests/test-batching.py
--- a/tests/test-batching.py
+++ b/tests/test-batching.py
@@ -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):
diff --git a/mercurial/wireprotov1peer.py b/mercurial/wireprotov1peer.py
--- a/mercurial/wireprotov1peer.py
+++ b/mercurial/wireprotov1peer.py
@@ -74,97 +74,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
@@ -364,9 +273,6 @@
# Begin of ipeercommands interface.
- def iterbatch(self):
- return remoteiterbatcher(self)
-
@batchable
def lookup(self, key):
self.requirecap('lookup', _('look up remote revision'))
diff --git a/mercurial/repository.py b/mercurial/repository.py
--- a/mercurial/repository.py
+++ b/mercurial/repository.py
@@ -274,29 +274,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."""
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -67,7 +67,6 @@
txnutil,
util,
vfs as vfsmod,
- wireprotov1peer,
)
from .utils import (
procutil,
@@ -155,20 +154,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):
@@ -334,9 +319,6 @@
finally:
executor.close()
- def iterbatch(self):
- return localiterbatcher(self)
-
# End of peer interface.
class locallegacypeer(repository.legacypeer, localpeer):
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list