-
Notifications
You must be signed in to change notification settings - Fork 13
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/add discovery client #3
Feat/add discovery client #3
Conversation
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.
It's a great start.
The FindPeers
method will need to be modified however so that it stores the cookie and iterates, otherwise it will always get just the first set of peers.
…egistered records. Default TTL for discovery client increased discovery client now utilizes server cookie for added efficiency
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.
This still needs work...
Also, can you call the discovery module file just discovery
? discovery_client
is a mouthful.
And try to shorten variable/type names as well.
No problem, I've found that longer names are more self descriptive (and not out of style in comparison to a number of Golang packages) and that with autocomplete they aren't actually a pain. However, I also have no interest in debating variable names as long as they are descriptive enough to get the point across so that interested reviewers can propose new names for them it's a simple approve from me. |
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.
this is looking good.
discovery.go
Outdated
} | ||
|
||
type discoveredPeerCache struct { | ||
cachedRecs map[peer.ID]*record |
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.
and this just recs
, it's a cache and these are named records.
@vyzo switched sync.Map with Map + RWMutex like we did in pubsub |
Added a stateful client that implements the libp2p discovery interface.