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: Accept handler #116

Merged
merged 3 commits into from
Nov 14, 2024
Merged

feat: Accept handler #116

merged 3 commits into from
Nov 14, 2024

Conversation

rklaehn
Copy link
Collaborator

@rklaehn rklaehn commented Nov 14, 2024

This adds a fn to RpcServer to run an accept loop, as well as a fn that spawns an accept loop.

This is to DRY the various places where we now have accept loops, such as

n0-computer/iroh-blobs#11
n0-computer/iroh-docs#7
n0-computer/iroh-gossip#7

This also allows making the tests a bit shorter - you don't have to call abort explicitly anymore on the server JoinHandle.

It doesn't do much, but saves quite a bit of boilerplate at the call site,
and also avoids having the task leaking by using AbortOnDropHandle.
Copy link
Contributor

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

very nice

@rklaehn rklaehn merged commit 32d5bc1 into main Nov 14, 2024
14 of 15 checks passed
@rklaehn rklaehn deleted the accept-handler branch November 14, 2024 16:05
@@ -162,7 +164,7 @@ async fn quinn_channel_smoke() -> anyhow::Result<()> {
///
/// This is a regression test.
#[tokio::test]
async fn server_away_and_back() -> anyhow::Result<()> {
async fn server_away_and_back() -> TestResult<()> {
Copy link
Member

Choose a reason for hiding this comment

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

Eeeeh too late but

Suggested change
async fn server_away_and_back() -> TestResult<()> {
async fn server_away_and_back() -> TestResult {

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