Introduce Prefetch trait #595
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
had a problem deploying
to
PR integration tests
GitHub Actions
Failure
Comment on lines
+37
to
+40
| type PrefetchResult<Client: ObjectClient + Send + Sync + 'static>: PrefetchResult<Client>; | ||
|
|
||
| /// Start a new prefetch request to the specified object. | ||
| fn prefetch<Client>( |
There was a problem hiding this comment.
Not going to block on it because we can refactor it later, but the Client type is a little weird here -- it's saying that you can pass any client into any prefetcher, but in reality it probably makes sense for a prefetcher to be coupled to a particular client once it's constructed.
I think in practice we want trait Prefetch<Client> or something, but that's a bit of annoying change to make.
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
|
The send constraint might just be a nightly regression (rust-lang/rust#117602), maybe check again tomorrow. |
Introduce a new `Prefetch` trait to abstract how `S3Filesystem` fetches object data from an `ObjectClient`. While this change does not introduce any functional change, this abstraction will be used to implement optional object data caching. The existing `Prefetcher` struct has been adapted to implement the new `Prefetch` trait. The main changes are: * it is generic on the `ObjectPartStream` (previously `ObjectPartFeed`), rather than using dynamic dispatch, * it does not own an `ObjectClient` instance, instead one is required when initiating a `prefetch` request, * the logic to spawn a new task for each `GetObject` request and handle the object body parts returned was moved into `ObjectPartStream`. Signed-off-by: Alessandro Passaro <[email protected]>
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
Right. It's been fixed now. I reverted the changes and rebased. |
jamesbornholt
approved these changes
Nov 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of change
Introduce a new
Prefetchtrait to abstract howS3Filesystemfetches object data from anObjectClient. While this change does not introduce any functional change, this abstraction will be used to implement optional object data caching.The existing
Prefetcherstruct has been adapted to implement the newPrefetchtrait. The main changes are:ObjectPartStream(previouslyObjectPartFeed), rather than using dynamic dispatch,ObjectClientinstance, instead one is required when initiating aprefetchrequest,GetObjectrequest and handle the object body parts returned was moved intoObjectPartStream.Relevant issues: #255
Does this change impact existing behavior?
No changes in behavior.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).