Skip to content
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

Merged
merged 5 commits into from
Jul 2, 2024

Conversation

dignifiedquire
Copy link
Contributor

@dignifiedquire dignifiedquire commented Jun 28, 2024

  • regular tests
  • netsim (nothing to do, it uses its own local relays)
  • CLI integration tests

Breaking Changes

  • iroh_net::defaults is now split into prod and staging
  • added iroh_net::RelayMode::Staging
  • iroh_net::discovery::dns:: N0_DNS_NODE_ORIGIN is now N0_DNS_NODE_ORIGIN_PROD and there is now N0_DNS_NODE_ORIGIN_STAGING

Closes #2420

@dignifiedquire dignifiedquire added this to the v0.20.0 milestone Jun 28, 2024
@dignifiedquire dignifiedquire marked this pull request as ready for review July 1, 2024 14:59
@dignifiedquire
Copy link
Contributor Author

I am not going to do the CLI tests, they are a pain, and are about to be refactored anyway

Copy link
Contributor

@flub flub left a 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?

Comment on lines 17 to 20
/// Use the default relay map, with production relay servers from n0.
DefaultProd,
/// Use the default relay map, with staging relay servers from n0.
DefaultStaging,
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it

@dignifiedquire dignifiedquire added this pull request to the merge queue Jul 2, 2024
Merged via the queue into main with commit d421ece Jul 2, 2024
25 of 26 checks passed
@dignifiedquire dignifiedquire deleted the feat-staging-prod-relays branch July 2, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

test: convert tests to use new test relay servers instead of prod servers
3 participants