-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remote_storage: add list_streaming API call #8466
Conversation
The two functions call each other: at least *one* of the two must be implemented by implementors (we recommend list_streaming).
3126 tests run: 3005 passed, 0 failed, 121 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
73741ec at 2024-07-23T18:03:37.197Z :recycle: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'm not a big fan of async_stream::stream
. fmt + clippy cannot check the async stream body :( I used https://crates.io/crates/futures-async-stream in the past but it's only on Rust nightly.
Taking a look at this, it seems that the azure SDK resets the internal state to The other relevant file to look at is The good news is that I see a path towards being able to do retries on Azure in a not too ugly way:
|
Implements the TODO from #8466 about retries: now the user of the stream returned by `list_streaming` is able to obtain the next item in the stream as often as they want, and retry it if it is an error. Also adds extends the test for paginated listing to include a dedicated test for `list_streaming`. follow-up of #8466 fixes #8457 part of #7547 --------- Co-authored-by: Joonas Koivunen <joonas@neon.tech>
This adds the ability to list many prefixes in a streaming fashion to the
RemoteStorage
crate as well asGenericRemoteStorage
.list
function of theRemoteStorage
trait is implemented by default in terms oflist_streaming
.list_streaming
is implemented and the defaultlist
implementation is used.LocalFs
, we keep thelist
implementation and makelist_streaming
call it.The
list_streaming
function is implemented for both S3 and Azure.A TODO for later is retries, which the scrubber currently has while the
list_streaming
implementations lack them.part of #8457 and #7547