This is an archive of the discontinued Mercurial Phabricator instance.

Differential D317

wireproto: properly implement batchable checking
ClosedPublic

Authored by indygreg on Aug 10 2017, 2:47 AM.

Details

Reviewers
quark
Group Reviewers
hg-reviewers
Commits
rHG297d1b70685c: wireproto: properly implement batchable checking
Summary

remoteiterbatcher (unlike remotebatcher) only supports batchable
commands. This claim can be validated by comparing their
implementations of submit() and noting how remoteiterbatcher assumes
the invoked method has a "batchable" attribute, which is set by
@peer.batchable.

remoteiterbatcher has a custom getitem that was trying to
validate that only batchable methods are called. However, it was only
validating that the called method exists, not that it is batchable.

This wasn't a big deal since remoteiterbatcher.submit() would raise
an AttributeError attempting to mtd.batchable(...).

Let's fix the check and convert it to ProgrammingError, which may
not have been around when this was originally implemented.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Aug 10 2017, 2:47 AM
quark accepted this revision.Aug 11 2017, 3:37 AM
Closed by commit rHG297d1b70685c: wireproto: properly implement batchable checking (authored by indygreg). · Explain WhyAug 11 2017, 2:51 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/wireproto.py (10 lines)

Diff 791

mercurial/wireproto.py

resref.set(next(batchable)) resref.set(next(batchable))
class remoteiterbatcher(peer.iterbatcher): class remoteiterbatcher(peer.iterbatcher):
def __init__(self, remote): def __init__(self, remote):
super(remoteiterbatcher, self).__init__() super(remoteiterbatcher, self).__init__()
self._remote = remote self._remote = remote
def __getattr__(self, name): def __getattr__(self, name):
if not getattr(self._remote, name, False): # Validate this method is batchable, since submit() only supports
raise AttributeError( # batchable methods.
'Attempted to iterbatch non-batchable call to %r' % name) 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) return super(remoteiterbatcher, self).__getattr__(name)
def submit(self): def submit(self):
"""Break the batch request into many patch calls and pipeline them. """Break the batch request into many patch calls and pipeline them.
This is mostly valuable over http where request sizes can be This is mostly valuable over http where request sizes can be
limited, but can be used in other places as well. limited, but can be used in other places as well.
""" """