-
Notifications
You must be signed in to change notification settings - Fork 261
feat: improve transfer, publish, resolve examples #3296
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/pr/3296/docs/iroh/ Last updated: 2025-05-12T07:31:16Z |
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.
Very nice!
@@ -404,3 +472,16 @@ fn parse_byte_size(s: &str) -> Result<u64> { | |||
let cfg = parse_size::Config::new().with_binary(); | |||
cfg.parse_size(s).map_err(|e| anyhow::anyhow!(e)) | |||
} | |||
|
|||
fn watch_conn_type(endpoint: &Endpoint, node_id: NodeId) -> AbortOnDropHandle<()> { | |||
let mut stream = endpoint.conn_type(node_id).unwrap().stream(); |
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.
Not a PR review comment, rather out API here isn't very nice. It seems this Result
that is unwrapped only exists because the NodeId
might not be known.
- At the very least this should be an
Option
, not a result. - Why not make it return pending forever? Or rather until that node becomes known. This could be either a footgun or a nice feature that you can start watching before the NodeId is known to the magic socket. And because a NodeId can be "forgotten" by the magic socket, and that doesn't suddenly result in an error at this point, it already behaves like this.
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.
Turning this into an issue: #3300
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.
Yeah, this is very true, it should just return a stream that waits for the node id to appear.
Because currently our API does not allow to match all conn type changes, we regularily miss the first change: I tried to create the conn_type
stream before calling connect
but then the unwrap
hits. And if calling after connect (as we do atm) then the first conn type change (from none
to relay
or mixed
) never appears.
Note: This needs updates to chuck because the transfer example is used for netsim. Tracking this here: n0-computer/chuck#75 |
|
Description
This improves the
transfer
,publish
andresolve
examples, which we often use for dogfooding new releases.transfer
example:prod
orstaging
environments with a single arg, while still optionally allowing to override relay, pkarr, and DNS servers.dev
environment that uses locally runningiroh-relay
andiroh-dns-server
instances, for easily using the example with a locally running dev setup of all services.IROH_SECRET
environment variableHere's the new help text and output:
Help text for provide
Help text for fetch
Output on provide side
Output on fetch side
publish
andresolve
examplesno-relay-url
allows to publish without relay URL.transfer
exampleBreaking Changes
Notes & open questions
Change checklist