Skip to content

feat: iroh-services relay preset_builder#81

Open
b5 wants to merge 4 commits into
mainfrom
b5/relay-constructor
Open

feat: iroh-services relay preset_builder#81
b5 wants to merge 4 commits into
mainfrom
b5/relay-constructor

Conversation

@b5
Copy link
Copy Markdown
Member

@b5 b5 commented May 4, 2026

Description

We should wait to merge this until upstream work is completed in iroh services.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@b5 b5 force-pushed the b5/relay-constructor branch from 6756405 to 16e14d7 Compare May 16, 2026 14:47
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2026

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-services/pr/81/docs/iroh_services/

Last updated: 2026-05-20T10:14:08Z

@b5 b5 force-pushed the b5/relay-constructor branch from 16e14d7 to 8ac0069 Compare May 16, 2026 14:49
@b5 b5 force-pushed the b5/relay-constructor branch from 8ac0069 to a45dedb Compare May 16, 2026 14:50
Comment thread Cargo.toml
@b5 b5 marked this pull request as ready for review May 19, 2026 15:52
@b5 b5 requested a review from dignifiedquire May 19, 2026 15:52
Comment thread examples/relays.rs
// pro & enterprise projects have access to custom relays, which are set
// with the `relays` method on the builder.
// You'll need to replace these strings with the relay urls for your project.
let _preset = iroh_services::preset()
Copy link
Copy Markdown
Contributor

@okdistribute okdistribute May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this example actually runnable? It seems like all of the other examples in this repository can be run by setting the API SECRET in env without modifying the code.

I'd prefer to keep it that way, and instead put this information in the documentation (https://docs.iroh.computer) and link to it from the README of this repository as well as the relays page on iroh services.

Copy link
Copy Markdown
Member Author

@b5 b5 May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep it's runnable! Or at least, it is on my dev branch & should be generally. I added a note about setting the IROH_SERVICES_API_SECRET in the example description in c124d31

Comment thread src/caps.rs Outdated
use serde::{Deserialize, Serialize};

/// How long tokens are valid for by default
pub(crate) const DEFAULT_CAP_EXPIRY: Duration = Duration::from_secs(60 * 60 * 24 * 30); // 1 month
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use Duration::from_days?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no such method I'm afraid, but I can bump up to Duration::from_hours

Comment thread src/lib.rs
api_secret::{API_SECRET_ENV_VAR_NAME, ApiSecret},
client::{Client, ClientBuilder},
net_diagnostics::{DiagnosticsReport, checks::run_diagnostics},
preset::{IrohServicesPreset, PresetBuilder, preset},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we have the bad precedent, but having a pub mod and pub use is really confusing

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this fully addresses it, but I've made preset private, and just exported the main types. Is that better?

Comment thread src/preset.rs Outdated
preset()
}

/// Returns the [`ApiSecret`] stashed on this preset, if one was set.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stashed is very odd wording

Comment thread src/preset.rs
pub fn client_builder(&self, endpoint: &Endpoint) -> ClientBuilder {
ClientBuilder::new(endpoint)
.api_secret(self.api_secret.clone())
.unwrap()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why unwrap?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you dig into the builder, the api_secret method on the builder has been factored to the point that it can no longer fail. I'll add a comment as such at a bare minimum, but can also either make this bubble up a Result, or we can break the API one last time, your call

Comment thread src/preset.rs
self
}

pub fn relays<I, S>(mut self, relays: I) -> Result<Self>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing docs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added an example to help make the generic argument more understandable

@b5 b5 force-pushed the b5/relay-constructor branch from c732552 to 275a54e Compare May 20, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants