Simplify remote store load_bytes API #18034
Merged
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 an no-functionality-change refactoring of
store::remote::ByteStore::load_bytes_with
that's arguably cleaner and also step towards #11149. In particular:load_bytes(...) -> Result<Option<Bytes>, String>
load_bytes
method itself, which is both easier to use, and keeps implementation details like gRPC (previously exposed as theByteStoreError::Grpc
/tonic::Status
error variant) entirely contained tostore::remote::ByteStore
ByteStoreError
enum can thus become private, because it's an implementation detail ofstore::remote::ByteStore
, no longer exposed in the public APIStep 1 resolves (and removes) a TODO comment. That TODO references #17065, but this patch doesn't fix that issue.
(NB. a significant fraction of the diff is just reindenting after change the nesting level.)
Background: this gets closer to #11149 by making it easier to factor out a "provider" trait:
store::remote::ByteStore
stores anArc<dyn ByteStoreProvider>
, where theByteStoreProvider
trait might be something like the following, implemented for the REAPI gRPC services and also for HTTP backends (as sketched in #17840):This trait needs to avoid generic methods (like the old
load_bytes_with
), so that it can be used as adyn
trait object.