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!: simplify LocalPool handling #47

Merged
merged 2 commits into from
Jan 21, 2025
Merged

Conversation

dignifiedquire
Copy link
Collaborator

Description

It has been a footgun for a few users, including myself, to make sure to keep around the LocalPool. This changes the behaviour to construct a LocalPool and keep it around by default. If necessary, in the builder one can provide a custom handle, if there is the need for a custom pool.

Breaking Changes

  • remove net_protocol::Blobs::new, use the builder instead
  • remove the LocalPoolHandle argument from net_protocol::Builder::build

Notes & open questions

Change checklist

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

Copy link

github-actions bot commented Jan 17, 2025

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

Last updated: 2025-01-17T09:57:28Z

It has been a footgun for a few users, including myself, to make sure to keep around the `LocalPool`.
This changes the behaviour to construct a `LocalPool` and keep it around by default. If necessary, in the builder one can provide a custom handle, if there is the need for a custom pool.
@dignifiedquire dignifiedquire force-pushed the feat-simplify-local-pool branch from 6ed5ca7 to 5e56fb0 Compare January 17, 2025 09:55
Comment on lines +171 to +172
.map(Rt::Handle)
.unwrap_or_else(|| Rt::Owned(LocalPool::default()));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.map(Rt::Handle)
.unwrap_or_else(|| Rt::Owned(LocalPool::default()));
.map_or_else(|| Rt::Owned(LocalPool::default()), Rt::Handle);

But like... could also be a match statement.

Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

IMHO this is an obvious improvement, so I'd love to see it land.

@dignifiedquire dignifiedquire merged commit b29991d into main Jan 21, 2025
23 of 24 checks passed
@dignifiedquire dignifiedquire deleted the feat-simplify-local-pool branch January 21, 2025 09:05
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