-
Notifications
You must be signed in to change notification settings - Fork 184
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!: split relay configuration between production and staging #2425
Conversation
b265f37
to
4e0003c
Compare
I am not going to do the CLI tests, they are a pain, and are about to be refactored anyway |
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.
Love it when I do a review and than don't hit submit...
LGTM I think, can't immediately think of something better.
The main consequence is that anyone else their test suite will also use the staging servers. That means we have to treat them as as important as the main ones. But otherwise probably the desired effect?
iroh-net/src/relay/map.rs
Outdated
/// Use the default relay map, with production relay servers from n0. | ||
DefaultProd, | ||
/// Use the default relay map, with staging relay servers from n0. | ||
DefaultStaging, |
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.
You have the option of calling this N0Prod
and N0Staging
since you're breaking the API anyway. You could also not break the API by doing Default
and N0Staging
or so. But this is fine as well.
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.
@ramfox any thoughts on which name to pick here?
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.
Just throwing in Default
& Staging
into the bikeshed. It's weird to have multiple "default".
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 it
CLI integration testsBreaking Changes
iroh_net::defaults
is now split intoprod
andstaging
iroh_net::RelayMode::Staging
iroh_net::discovery::dns:: N0_DNS_NODE_ORIGIN
is nowN0_DNS_NODE_ORIGIN_PROD
and there is nowN0_DNS_NODE_ORIGIN_STAGING
Closes #2420