This is an archive of the discontinued Mercurial Phabricator instance.

simplestore: shore up lookup errors
ClosedPublic

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

Details

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

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

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.

This revision was automatically updated to reflect the committed changes.