-
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: allow custom blob providing event handling #2583
Merged
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
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2583/docs/iroh/ Last updated: 2024-08-13T09:25:48Z |
rklaehn
reviewed
Aug 6, 2024
# Conflicts: # iroh/src/client/blobs.rs
we still have a special case for no event sender so we don't pay the cost for that if we don't need it.
rklaehn
force-pushed
the
feat-blob-provider-events
branch
from
August 12, 2024 11:57
5311b5d
to
a9881d3
Compare
Also take events by ref so we can avoid an arc clone. The SendingSliceReader takes a mutable reference to the inner reader anyway, so it is not 'static.
rklaehn
force-pushed
the
feat-blob-provider-events
branch
from
August 13, 2024 09:25
8a7cd62
to
1ac4e20
Compare
dignifiedquire
commented
Aug 13, 2024
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
rklaehn
approved these changes
Aug 13, 2024
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
This adds the ability back to track blob provide events from an iroh node.
Breaking Changes
Event
enum:EventSender
has been renamedCustomEventSender
EventSender
Notes & open questions
Notes:
I added progress events that get sent every single 16 KiB chunk. These events contain the usual data (connection id and request id) plus offset and hash. Adding the hash makes the event larger, but makes it very much self-contained.
The event is fired after the read, but that does not mean that the data has been successfully transferred. Should be good enough to show progress anyway.
These will be a lot of events, so they are sent using try_send in order not to slow down the transfer. That means that you might miss some events, but since they only contain the end_offset that should be fine.
Open questions:
I called the trait CustomEventSender and the concrete handle for an optional CustomEventSender just
EventSender
. I can revert this if somebody can think of a better name for the trait or the handle...Change checklist