Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the one change I'm fairly confident ought to happen in storage: factoring the
get
paths into a common funnel. Besides being duplicate code, if you look carefully the previous arrangement had inconsistent handling ofNone
returns from the underlying map too: one path (correctly) treated it as an internal error sinceprepare_read_only_access
shouldn't allow getting there; the other two paths either returned it as-is or merged it into theMissingValue
path.I'm not changing two other error codes just yet, though I was a bit tempted to. Would be interested to hear input from @dmkozh : as mentioned in conversation elsewhere, I think the footprint checking paths should probably return
InvalidAction
as errors rather thanExceededLimit
; and possibly the path inextend
that returns anInternalError
live-until value should beInvalidAction
too. But I can imagine the counterarguments to these, and if you feel strongly that they're correct already I'm ok leaving them as-is.