-
Notifications
You must be signed in to change notification settings - Fork 21
docs: add example for creating & fetching a collection #176
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
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/176/docs/iroh_blobs/ Last updated: 2025-10-07T07:04:05Z |
cc64450 to
703fd2a
Compare
703fd2a to
8a00f47
Compare
examples/transfer-collection.rs
Outdated
|
|
||
| impl Node { | ||
| async fn new() -> Result<Self> { | ||
| let endpoint = Endpoint::builder().bind().await?; |
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'm skipping .discovery_n0() here and using add_node_addr directly on the endpoint in the Node::get_collection method below. Largely to take a highligher to the add_node_addr discussion we've been having.
@rklaehn: the store has a connection pool, but I can't seem to get at it? Seems the connection pool relies on me being able to plumb addresses into the endpoint, is that right?
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.
The store doesn't have anything, not even an endpoint. The downloader has a connection pool.
| /// retrive an entire collection from a given hash and provider | ||
| async fn get_collection(&self, hash: Hash, provider: NodeAddr) -> Result<()> { | ||
| self.router.endpoint().add_node_addr(provider.clone())?; | ||
| let req = HashAndFormat::hash_seq(hash); |
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.
This magical incantation to feed to download as a req param was super tough to figure out. Maybe we just handle this with examples.
| async fn get_collection(&self, hash: Hash, provider: NodeAddr) -> Result<()> { | ||
| self.router.endpoint().add_node_addr(provider.clone())?; | ||
| let req = HashAndFormat::hash_seq(hash); | ||
| let addrs = Shuffled::new(vec![provider.node_id]); |
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 was surprised while working with store.downloader().download()that theContentDiscoverytrait worked inNodeIdand notNodeAddr`, is there a reason why?
Second: would it be possible to set it up so I can pass a Vec<NodeId> / Vec<NodeAddr>, and call .into() on it to get a Shuffled content discovery? That might help remove the concept of Content Discovery entirely from what we'd pass to download.
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 was surprised while working with store.downloader().download()that theContentDiscoverytrait worked inNodeIdand notNodeAddr`, is there a reason why?
I think it is nicer to let the downloader deal only with node ids and let the node discovery figure out how to dial the nodes.
I guess you could use NodeAddr in the public API, and then just call endpoint.add_node_addr before handing it over to the lower levels. But I am very much convinced that the lower levels should not do NodeAddrs.
Any large scale content discovery mechanism will only ever work with node ids, since node ids are longer time stable than addrs.
Second: would it be possible to set it up so I can pass a Vec / Vec, and call .into() on it to get a Shuffled content discovery? That might help remove the concept of Content Discovery entirely from what we'd pass to download.
I don't quite get this. You don't always want shuffled. And since ContentDiscovery itself is an infinite stream of node ids you can't make this a combinator - no way to shuffle an infinite stream.
I guess we could maybe make a builder for the common case where you have a finite set of node ids, where you would have shuffled as a combinator.
| self.store | ||
| .downloader(self.router.endpoint()) |
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'm super confused about why I have to re-pass the endpoint to the store here to create a downloader. I guess I'm supposed to make a downloader & store it on the Node struct I've defined here?
It generally feels like a higher-level API is missing that just falls back to a DefaultDownloader
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.
The store can be used without an endpoint. There are many very valid use cases for this, e.g. you have a local store and want to do something without ever listening on the network.
I guess you could do a thing which combines a store and an endpoint, but then that is what the downloader is. Maybe there needs to be a thing that combines a store, endpoint and downloader.
I want to keep the downloader as a separate thing though since it has some internal state and there might be situations where you want to create/destroy them or even have multiple independent of the store.
rklaehn
left a comment
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.
All fine except for that one bug with the collection tag.
Description
Breaking Changes
Notes & open questions
Change checklist