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: override to staging relays #2551

Merged
merged 6 commits into from
Jul 28, 2024
Merged

feat: override to staging relays #2551

merged 6 commits into from
Jul 28, 2024

Conversation

Arqu
Copy link
Collaborator

@Arqu Arqu commented Jul 25, 2024

Description

While I'm not a huge fan of this piece of code, I don't really see much of an alternative as we want to be able to force the override to use staging relays even in "prod" environments or when it's deeply integrated such as the FFI library cases.

This should allow us to override the default relay maps for all of our CI setups in all of our repos.

Breaking Changes

Notes & open questions

Change checklist

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

@Arqu Arqu requested a review from dignifiedquire July 25, 2024 19:37
@Arqu Arqu self-assigned this Jul 25, 2024
@@ -59,6 +59,9 @@ pub use iroh_base::node_addr::{AddrInfo, NodeAddr};
/// is still no connection the configured [`Discovery`] will be used however.
const DISCOVERY_WAIT_PERIOD: Duration = Duration::from_millis(500);

/// Environment variable to force the use of staging relays.
pub const ENV_FORCE_STAGING_RELAYS: &str = "IROH_FORCE_STAGING_RELAYS";
Copy link
Contributor

Choose a reason for hiding this comment

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

if you put the env var in here, I would also put the check in here and make it public function

Copy link
Contributor

Choose a reason for hiding this comment

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

does this still need pub?

default_eu_relay_node(),
default_ap_relay_node(),
]
.into()
Copy link
Contributor

Choose a reason for hiding this comment

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

this should just be a function from iroh_net, to avoid this duplication

@@ -133,6 +145,17 @@ pub struct RelayNode {
pub stun_port: u16,
}

impl RelayNode {
/// Creates a new relay node from itself.
pub fn copied(&self) -> Self {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know what's the expected naming or way to do this properly. I just needed to get it out of the arc and this was cheap and easy enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

no need for this method, just use this to get it out of the arc: https://doc.rust-lang.org/std/sync/struct.Arc.html#method.unwrap_or_clone

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Figured there was a saner way to do this.

@Arqu Arqu enabled auto-merge July 28, 2024 21:38
Copy link

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

Last updated: 2024-07-28T21:38:18Z

@Arqu Arqu added this pull request to the merge queue Jul 28, 2024
Merged via the queue into main with commit ed4420b Jul 28, 2024
28 checks passed
@Arqu Arqu deleted the arqu/staging_latch branch July 28, 2024 21:58
@Arqu Arqu mentioned this pull request Jul 29, 2024
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Jul 29, 2024
## Description

Now that #2551 is landed, time
to force all our repos to set this env var.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
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.

2 participants