refactor(iroh-net)!: Improve server modules structure & rename structs #2568

matheus23 · 2024-07-30T11:54:53Z

Description

This is moving around the relay modules a bit. Mostly so that all server-specific code is under iroh-net/src/relay/server/.
This means there's only a handful of #[cfg(feature = "iroh-relay")]s in relay.rs and a bunch of other places due to unused warnings otherwise.

I've also renamed what was previously iroh::relay::client::{Client, ClientBuilder, ClientReceiver, ...} to iroh::relay::client::conn::{Conn, ConnBuilder, ConnReceiver, ...}, because otherwise the Client* names were duplicated in two modules that were in a parent-child relationship, which is super confusing.
And really, the inner Clients are more about individual connections.

I also resolved a TODO around moving to an AbortingJoinHandle in favor of a custom RelayHttpServerGuard.

Breaking Changes

Refactored the module structure for users of the iroh-relay feature in iroh-net
- moved iroh_net::relay::iroh_relay::* to iroh_net::relay::server::*
- moved iroh_net::relay::ClientConnHandler to iroh_net::relay::server::ClientConnHandler
- moved iroh_net::relay::Metrics to iroh_net::relay::server::Metrics
- moved iroh_net::relay::MaybeTlsStreamServer to iroh_net::relay::server::MaybeTlsStreamServer
- removed iroh_net::relay::http::ServerHandle. The server can now be aborted via its task_handle() function or by dropping it.
- renamed iroh_net::relay::RelayClient to iroh_net::relay::RelayConn
- moved iroh_net::relay::http::Client to iroh_net::relay::HttpClient
- moved iroh_net::relay::http::ClientBuilder to iroh_net::relay::HttpClientBuilder
- moved iroh_net::relay::http::ClientReceiver to iroh_net::relay::HttpClientReceiver
- moved iroh_net::relay::http::ClientError to iroh_net::relay::HttpClientError
- moved iroh_net::relay::http::TlsConfig to iroh_net::relay::server::TlsConfig
- renamed and moved iroh_net::relay::Server to iroh_net::relay::server::ServerActorTask
- unexposed iroh_net::relay::http::{Server, ServerBuilder}. Use iroh_net::relay::server::Server instead.
Properly feature-gate the iroh server implementation behind #[cfg(feature = "iroh-relay")] by feature-gating the whole iroh_net::relay::server module.

Notes & open questions

I realize the diff here is quite substantial. Unfortunately the diff is mostly just bad because I ended up "swapping places" for modules in two cases, which ended up creating really bad diffs.

I know the diff is quite sad, so it's fair if we say "this is unreviewable" and I scrap this piece of work. I'm not blocked on this merging, and I only fairly quickly knocked this out today.
As Mark Twain famously said

I apologize for such a long PR - I didn't have time to write a short one.

Or sth like that.

Change checklist

Self-review.
Documentation updates following the style guide, if relevant.
~~[ ] Tests if relevant.~~
All breaking changes documented. Yeah? Kinda?

github-actions · 2024-07-30T14:01:15Z

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

Last updated: 2024-07-31T09:13:09Z

matheus23 · 2024-07-30T15:58:49Z

iroh-net/src/relay/client.rs

-//! based on tailscale/derp/derp_client.go
-use std::net::SocketAddr;
-use std::pin::Pin;
+//! Based on tailscale/derp/derphttp/derphttp_client.go


As you can see, the diff here is fucked 😬

I moved relay/http/client.rs to here, and moved the old relay/client.rs to relay/client/conn.rs.
I also renamed a bunch of the Client* things to Conn*, which probably made this diff look bad.

matheus23 · 2024-07-30T16:02:30Z

iroh-net/src/relay/server.rs

-//! based on tailscale/derp/derp_server.go
-use std::collections::HashMap;
+//! A full-fledged iroh-relay server.


Again, the diff here ended up bad. I think it may actually be due to the fact that I've moved this module somewhere else while simultaneously moving something else to here :/

matheus23 · 2024-07-30T16:06:02Z

iroh-net/src/relay/server/http_server.rs

+#[cfg(test)]
+mod tests {


I moved these tests to here from relay/http.rs, as that module mainly included types and constants, not the code.

matheus23 · 2024-07-30T16:12:01Z

There's some small unused things remaining. Will update the PR tomorrow morning ✌️

In the mean time - I've written up some notes in the PR description.

dignifiedquire · 2024-07-30T20:44:16Z

seems mostly fine from my side, @ramfox is the master of this code though, so she will have to deliver final verdict

ramfox

Okay, went through this. Looks good. I especially like the rename from Client to Conn. I think client::Conn and server::ClientConn mirror each other better now. It makes it a bit more clear that the server::ClientConn is the server-side driver of the connection to the client.

Approved! Nice work! Happy to re-review on the last bits of clean up when you need it.

matheus23 · 2024-07-31T07:36:48Z

Current state of this is:
I need #2566 to be reviewed and then merged first, because this depends on that.

flub

nice restructure!

looking forward to merging this into the quinn11 branch 🙃

…ay::server::service` to `relay::server`

…rs` one level up

…ver`

…` into `codec`

matheus23 self-assigned this Jul 30, 2024

matheus23 changed the title ~~refactor(iroh-net)!: Move around relay server modules & rename structs~~ refactor(iroh-net)!: Improve server modules structure & rename structs Jul 30, 2024

matheus23 force-pushed the matheus23/more-cfg-iroh-relay branch from 5521564 to 9ac0be6 Compare July 30, 2024 11:57

matheus23 force-pushed the matheus23/refactor-relay-server branch from d38a4ac to 88c3019 Compare July 30, 2024 12:04

matheus23 marked this pull request as ready for review July 30, 2024 15:34

matheus23 commented Jul 30, 2024

View reviewed changes

dignifiedquire requested a review from ramfox July 30, 2024 20:43

ramfox approved these changes Jul 30, 2024

View reviewed changes

flub approved these changes Jul 31, 2024

View reviewed changes

matheus23 changed the base branch from matheus23/more-cfg-iroh-relay to main July 31, 2024 09:11

matheus23 added 14 commits July 31, 2024 11:12

feat(iroh-net)!: Move more server code behind iroh-relay feature flag

6286fb3

fix(iroh-net): Imports and remove dead code

88a128a

Don't actually hide ReceivedMessage

207a43a

Undo some unexposing

5d08f5f

Move server-related modules below the server submodule

af9684b

Use AbortingJoinHandle in favor of JoinHandle

959b0ff

refactor: Rename server::Server -> ServerActorTask

6d2927c

Fix documentation, make ServerActor module-private

39c4e41

Move relay::server module into relay::server::actor and move `rel…

5c7c1df

…ay::server::service` to `relay::server`

Use alias in bin/iroh-relay.rs for relay::server module

94c7608

Rename all things relay::client::Client* to Conn*

8177cfb

Rename relay/client.rs to relay/conn.rs

7622114

Move relay/conn.rs to relay/client/conn.rs and move `http/client.…

fc7fb31

…rs` one level up

Fix more docs & exposes

5f38e9d

matheus23 added 7 commits July 31, 2024 11:12

Move relay/http/streams to relay/client/streams

dff7929

Move relay/http/server into relay/server/http_server

89c05ca

Remove RelayHttpServerGuard and move its logic to `http_server::Ser…

998679a

…ver`

Fix documentation references

f47298e

Move relay/types to relay/server/types (and split out `ClientInfo…

c65444b

…` into `codec`

Delete unused/mark things as unused

9410298

More module docs & move relay metrics to server::metrics

6c818cb

matheus23 force-pushed the matheus23/refactor-relay-server branch from f19fcc7 to 6c818cb Compare July 31, 2024 09:13

matheus23 enabled auto-merge July 31, 2024 09:13

matheus23 added this pull request to the merge queue Jul 31, 2024

Merged via the queue into main with commit 29d2e82 Jul 31, 2024
27 of 28 checks passed

dignifiedquire deleted the matheus23/refactor-relay-server branch July 31, 2024 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(iroh-net)!: Improve server modules structure & rename structs #2568

refactor(iroh-net)!: Improve server modules structure & rename structs #2568

matheus23 commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

matheus23 Jul 30, 2024

matheus23 Jul 30, 2024

matheus23 Jul 30, 2024

matheus23 commented Jul 30, 2024

dignifiedquire commented Jul 30, 2024

ramfox left a comment •

edited

Loading

matheus23 commented Jul 31, 2024

flub left a comment

		#[cfg(test)]
		mod tests {

refactor(iroh-net)!: Improve server modules structure & rename structs #2568

refactor(iroh-net)!: Improve server modules structure & rename structs #2568

Conversation

matheus23 commented Jul 30, 2024 • edited Loading

Description

Breaking Changes

Notes & open questions

Change checklist

github-actions bot commented Jul 30, 2024 • edited Loading

matheus23 Jul 30, 2024

Choose a reason for hiding this comment

matheus23 Jul 30, 2024

Choose a reason for hiding this comment

matheus23 Jul 30, 2024

Choose a reason for hiding this comment

matheus23 commented Jul 30, 2024

dignifiedquire commented Jul 30, 2024

ramfox left a comment • edited Loading

Choose a reason for hiding this comment

matheus23 commented Jul 31, 2024

flub left a comment

Choose a reason for hiding this comment

matheus23 commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

ramfox left a comment •

edited

Loading