-
Notifications
You must be signed in to change notification settings - Fork 165
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
feat(iroh)!: make blobs::read_at more flexible #2756
Conversation
Introduces `ReadAtLen` which allows to specify the behaviour of `read_at` more closely - `All` - reads until the end (formerly `None`) - `Exact(size)` - Reads exactly this many bytes (formerly `Some(size)` - `AtMost(size)` - Reads at most this many bytes, but allows for a shorter response if not enough data is available Closes #2738 ## Breaking Changes - `iroh::client::Blobs::read_at` and `read_at_to_bytes` now take `ReadAtLen` instead of `Option<usize>`
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.
Approved, but I think it would be worht a try to do the Into thing. Maybe it won't work for some reason, but it sure would be nicer for the standard use...
@@ -159,7 +159,7 @@ impl Client { | |||
/// Read offset + len from a single blob. | |||
/// | |||
/// If `len` is `None` it will read the full blob. | |||
pub async fn read_at(&self, hash: Hash, offset: u64, len: Option<usize>) -> Result<Reader> { | |||
pub async fn read_at(&self, hash: Hash, offset: u64, len: ReadAtLen) -> Result<Reader> { |
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.
I like using an enum, but for the public API can we maybe do something with a From conversion so that you can still provide just a length?
So this would be fn read_at(&self, hash: Hash, offset: u64, len: impl Into<ReadAtLen>)
and you would implement From for ReadAtLen to map to Exact?
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.
I don't want to do this, because i think it is confusing/not clear if the into behaviour will be exact or at_most.
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2756/docs/iroh/ Last updated: 2024-09-30T12:43:55Z |
Introduces `ReadAtLen` which allows to specify the behaviour of `read_at` more closely - `All` - reads until the end (formerly `None`) - `Exact(size)` - Reads exactly this many bytes (formerly `Some(size)` - `AtMost(size)` - Reads at most this many bytes, but allows for a shorter response if not enough data is available Closes #2738 ## Breaking Changes - `iroh::client::Blobs::read_at` and `read_at_to_bytes` now take `ReadAtLen` instead of `Option<usize>` --------- Co-authored-by: Asmir Avdicevic <[email protected]>
Introduces
ReadAtLen
which allows to specify the behaviour ofread_at
more closelyAll
- reads until the end (formerlyNone
)Exact(size)
- Reads exactly this many bytes (formerlySome(size)
AtMost(size)
- Reads at most this many bytes, but allows for a shorter response if not enough data is availableCloses #2738
Breaking Changes
iroh::client::Blobs::read_at
andread_at_to_bytes
now takeReadAtLen
instead ofOption<usize>