-
Notifications
You must be signed in to change notification settings - Fork 177
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: Make Endpoint::node_addr
watchable and add trait Watcher
& combinators
#3045
base: main
Are you sure you want to change the base?
Conversation
3b22ade
to
2529cde
Compare
/// Returns the currently held value. | ||
/// This handle allows one to observe the latest state and be notified | ||
/// of any changes to this value. | ||
pub trait Watcher: Clone { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of the DirectWatcher
name. I wonder if this name can be something else instead? Maybe ValueWatcher
? Or even WatcherTrait
? I don't know, just some random names that came out of my keyboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not super happy either.
I would like to keep the clean trait Watcher
name, since I'd rather have people use impl Watcher
in their APIs than surface DirectWatcher
... But of course that means no named types 😭
And I do think Value
doesn't actually help, since every watcher is a "value" watcher.
SourceWatcher
?
Not sure.
111a428
to
f8b7e5e
Compare
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3045/docs/iroh/ Last updated: 2024-12-19T16:06:33Z |
- Properly close the connection - Rename to `endpoint_conn_type_becomes_direct` - Use `TestResult` - Rename `handle_direct_connection` to `wait_for_conn_type_direct`
Description
Goal of this PR was: make
Endpoint::node_addr
return something that can be watched, so you can useep.node_addr().initialized().await?
orep.node_addr().get()?
orep.node_addr().updated().await?
, etc.Unfortunately, there's no
Watchable<Option<NodeAddr>>
anywhere, instead, aNodeAddr
technically consists of aWatchable<Option<BTreeSet<DirectAddr>>>
and aWatchable<Option<RelayUrl>>
.So this PR abstracts all the
initialized
,updated
, etc. functionality ofWatcher
into atrait Watcher
, and renameswatchable::Watcher
intowatcher::Direct
. (the wholewatchable
module was renamed towatcher
)On top of this we can build the usual combinators like
watcher::Map
that transforms aWatcher<Value = A>
into aWatcher<Value = B>
using a functionA -> B
, or the tuple(impl Watcher<Value = A>, impl Watcher<Value = B>)
that takes the two watchers and implementsWatcher<Value = (A, B)>
.Breaking Changes
watchable
module towatcher
watchable::Watcher::{get, updated, initialized, stream, stream_updates_only}
totrait watcher::Watcher
watchable::Watcher
towatcher::Direct
watchable::WatchNextFut
towatcher::NextFut
watchable::WatchInitializedFut
towatcher::InitializedFut
watchable::WatcherStream
towatcher::Stream
watchable::Watchable
towatcher::Watchable
watchable::Disconnected
towatcher::Disconnected
Notes & open questions
This PR would subsume #2732
You're probably wondering:
Why is
Watcher
a trait?Two reasons:
watcher::Map
or the watcher tuple are doing. The alternative is to have a concrete type for special cases per-api, like e.g. a customNodeAddrWatcher
that implements essentiallywatcher::Map
and watcher tuple functionality inside. This works, but I'd argue is less comprehensible ("What is the difference betweenNodeAddrWatcher
andwatcher::Direct
? They seem to have the same API?") and also less composable "What if I want to combine the Watchers further?".watcher::Direct
,watcher::Map
, etc. we can re-use the same default implementations of theWatcher
trait, instead of having to repeat these implementations for all watchers, potentially allowing them to get out of sync or behave differently from one another.What is the difference between
Stream
andWatcher
?The main difference is that
Watcher
combines a poll function for the next item with a functionget(&self) -> Result<Self::Value, Disconnected>
that allows you to always try to get the current value.This allows the
poll_updated
function (which is very similar toStream::poll_next
) to gain some superpowers, e.g. theOrWatcher
combinator canget
the value of the otherWatcher
when one of its watchers has an update.Change checklist