The remote batching code is difficult to read. Let's improve it.
As part of the refactor, the future returned by method calls on
batchiter() instances is now populated. However, you still need to
consume the results() generator for the future to be set. But at
least now we can stuff the future somewhere and not have to worry
about aligning method call order with result order since you can
use a future to hold the result.
Also as part of the change, we now verify that @batchable generators
yield exactly 2 values. In other words, we enforce their API.
The non-iter batcher has been unused since 3bcb9f9a4a63. And to my
surprise we had no explicit unit test coverage of it! test-batching.py
has been overhauled to use the iterating batcher.
Since the iterating batcher doesn't allow non-batchable method
calls nor local calls, tests have been updated to reflect reality.
The iterating batcher has been used for multiple releases apparently
without major issue. So this shouldn't cause alarm.
.. api::
@peer.batchable functions must now yield exactly 2 values
Differential Revision: https://phab.mercurial-scm.org/D319
@peer.batchable decorated generator functions have two forms:
yield value, None
and
yield args, future
yield value
These forms have been present since the decorator was introduced.
There are currently no in-repo consumers of the first form. So this
commit removes support for it.
Note that remoteiterbatcher.submit() asserts the 2nd form. And
3bcb9f9a4a63 removed the last user of remotebatcher, forcing everyone
to remoteiterbatcher. So anything relying on this in the wild would
have been broken since 3bcb9f9a4a63.
.. api::
@peer.batchable can no longer emit local values
Differential Revision: https://phab.mercurial-scm.org/D318
"Real" batching only makes sense for wirepeers, but it greatly
simplifies the clients of peer instances if they can be ignorant to
actual batching capabilities of that peer. By moving the
not-really-batched batching code into peer.peer, all peer instances
now work with the batching API, thus simplifying users.
This leaves a couple of name forwards in wirepeer.py. Originally I had
planned to clean those up, but it kind of unclarifies other bits of
code that want to use batching, so I think it makes sense for the
names to stay exposed by wireproto. Specifically, almost nothing is
currently aware of peer (see largefiles.proto for an example), so
making them be aware of the peer module *and* the wireproto module
seems like some abstraction leakage. I *think* the right long-term fix
would actually be to make wireproto an implementation detail that
clients wouldn't need to know about, but I don't really know what that
would entail at the moment.
As far as I'm aware, no clients of batching in third-party extensions
will need updating, which is nice icing.
