This is an archive of the discontinued Mercurial Phabricator instance.

Differential D608

wireproto: do not abort after successful lookup
ClosedPublic

Authored by spectral on Sep 1 2017, 5:00 PM.

Details

Reviewers
quark
Group Reviewers
hg-reviewers
Commits
rHG6c6169f71b8d: wireproto: do not abort after successful lookup
Summary

As far as I can tell, this interface originally used 'return' here, so the
"fallthrough" to self._abort made sense. When it was switched to 'yield' this
didn't make sense, but doesn't impact most uses because the 'plain' wrapper in
peer.py's 'batchable' decorator only attempts to yield two items (args and
value).

When using iterbatch, however, it attempts to verify that the @batchable
generators only emit 2 results, by expecting a StopIteration when attempting to
access a third.

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

spectral created this revision.Sep 1 2017, 5:00 PM
quark accepted this revision.Sep 1 2017, 8:23 PM
quark added a subscriber: quark.

LGTM. I have checked there are no other suspicious _abort in this file.

Closed by commit rHG6c6169f71b8d: wireproto: do not abort after successful lookup (authored by spectral). · Explain WhySep 3 2017, 10:20 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/wireproto.py (3 lines)

Diff 1588

mercurial/wireproto.py

def lookup(self, key): def lookup(self, key):
self.requirecap('lookup', _('look up remote revision')) self.requirecap('lookup', _('look up remote revision'))
f = future() f = future()
yield {'key': encoding.fromlocal(key)}, f yield {'key': encoding.fromlocal(key)}, f
d = f.value d = f.value
success, data = d[:-1].split(" ", 1) success, data = d[:-1].split(" ", 1)
if int(success): if int(success):
yield bin(data) yield bin(data)
else:
self._abort(error.RepoError(data)) self._abort(error.RepoError(data))
@batchable @batchable
def heads(self): def heads(self):
f = future() f = future()
yield {}, f yield {}, f
d = f.value d = f.value
try: try:
yield decodelist(d[:-1]) yield decodelist(d[:-1])