This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3092

simplestore: shore up lookup errors
ClosedPublic

Authored by indygreg on Apr 4 2018, 9:24 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGdd2753729853: simplestore: shore up lookup errors
Summary

When revisions or nodes can't be resolved, we're expected to raise
an error.LookupError. When I ported code from revlog.py, I failed
to realize that "LookupError" in that module is aliased to
error.LookupError. I thought we were using the builtin LookupError
instead.

This commit switches us to error.LookupError. It also fixes
rev() to raise error.LookupError instead of KeyError.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Apr 4 2018, 9:24 PM
martinvonz added a subscriber: martinvonz.Apr 5 2018, 1:31 AM

When revisions or nodes can't be resolved, we're expected to raise
an error.LookupError. When I ported code from revlog.py, I failed
to realize that "LookupError" in that module is aliased to
error.LookupError. I thought we were using the builtin LookupError
instead.

I've also recently been confused by that. Perhaps we should even rename error.LookupError to something else and avoid shadowing the builtin? StoreLookupError, perhaps?

durin42 added a subscriber: durin42.Apr 6 2018, 3:38 PM
In D3092#50126, @martinvonz wrote:

When revisions or nodes can't be resolved, we're expected to raise
an error.LookupError. When I ported code from revlog.py, I failed
to realize that "LookupError" in that module is aliased to
error.LookupError. I thought we were using the builtin LookupError
instead.

I've also recently been confused by that. Perhaps we should even rename error.LookupError to something else and avoid shadowing the builtin? StoreLookupError, perhaps?

Probably a good idea.

Closed by commit rHGdd2753729853: simplestore: shore up lookup errors (authored by indygreg). · Explain WhyApr 6 2018, 3:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mtests/simplestorerepo.py (19 lines)

Diff 7674

tests/simplestorerepo.py

def parentrevs(self, rev): def parentrevs(self, rev):
p1, p2 = self.parents(self._indexbyrev[rev][b'node']) p1, p2 = self.parents(self._indexbyrev[rev][b'node'])
return self.rev(p1), self.rev(p2) return self.rev(p1), self.rev(p2)
def rev(self, node): def rev(self, node):
validatenode(node) validatenode(node)
# Will raise KeyError. try:
self._indexbynode[node] self._indexbynode[node]
except KeyError:
raise error.LookupError(node, self._indexpath, _('no node'))
for rev, entry in self._indexbyrev.items(): for rev, entry in self._indexbyrev.items():
if entry[b'node'] == node: if entry[b'node'] == node:
return rev return rev
raise error.ProgrammingError('this should not occur') raise error.ProgrammingError('this should not occur')
def node(self, rev): def node(self, rev):
validaterev(rev) validaterev(rev)
return self._indexbyrev[rev][b'node'] return self._indexbyrev[rev][b'node']
def lookup(self, node): def lookup(self, node):
if isinstance(node, int): if isinstance(node, int):
return self.node(node) return self.node(node)
if len(node) == 20: if len(node) == 20:
try:
self.rev(node) self.rev(node)
return node return node
except LookupError:
pass
try: try:
rev = int(node) rev = int(node)
if '%d' % rev != node: if '%d' % rev != node:
raise ValueError raise ValueError
if rev < 0: if rev < 0:
rev = len(self) + rev rev = len(self) + rev
if rev < 0 or rev >= len(self): if rev < 0 or rev >= len(self):
raise ValueError raise ValueError
return self.node(rev) return self.node(rev)
except (ValueError, OverflowError): except (ValueError, OverflowError):
pass pass
if len(node) == 40: if len(node) == 40:
try: try:
rawnode = bin(node) rawnode = bin(node)
self.rev(rawnode) self.rev(rawnode)
return rawnode return rawnode
except (TypeError, LookupError): except TypeError:
pass pass
raise LookupError(node, self._path, _('invalid lookup input')) raise error.LookupError(node, self._path, _('invalid lookup input'))
def linkrev(self, rev): def linkrev(self, rev):
validaterev(rev) validaterev(rev)
return self._indexbyrev[rev][b'linkrev'] return self._indexbyrev[rev][b'linkrev']
def flags(self, rev): def flags(self, rev):
validaterev(rev) validaterev(rev)
self._path) self._path)
def revision(self, node, raw=False): def revision(self, node, raw=False):
validatenode(node) validatenode(node)
if node == nullid: if node == nullid:
return b'' return b''
self._indexbynode[node]
rev = self.rev(node) rev = self.rev(node)
flags = self.flags(rev) flags = self.flags(rev)
path = b'/'.join([self._storepath, hex(node)]) path = b'/'.join([self._storepath, hex(node)])
rawtext = self._svfs.read(path) rawtext = self._svfs.read(path)
text, validatehash = self._processflags(rawtext, flags, 'read', raw=raw) text, validatehash = self._processflags(rawtext, flags, 'read', raw=raw)
if validatehash: if validatehash: