Conversation
core/coreapi/swarm.go
Outdated
| } | ||
|
|
||
| func (ci *connInfo) ID() peer.ID { | ||
| return ci.ID() |
There was a problem hiding this comment.
Sorry for nitpick. This line will loop infinity 🐛
There was a problem hiding this comment.
That's not what I'd call a "nitpick".
efe5033 to
8333fa8
Compare
Stebalien
left a comment
There was a problem hiding this comment.
I'd like to port the swarm command over to this interface before merging to make sure it does everything we need (and to test it). Will that blow up the patch too much?
core/coreapi/interface/swarm.go
Outdated
| Latency(context.Context) (time.Duration, error) | ||
|
|
||
| // Streams returns list of streams established with the peer | ||
| // TODO: should this return multicodecs? |
core/coreapi/interface/swarm.go
Outdated
| ) | ||
|
|
||
| // PeerInfo contains information about a peer | ||
| type PeerInfo interface { |
There was a problem hiding this comment.
Is this PeerInfo or ConnectionInfo? It looks like this is a per-connection datastructure.
core/coreapi/interface/swarm.go
Outdated
| // SwarmAPI specifies the interface to libp2p swarm | ||
| type SwarmAPI interface { | ||
| // Connect to a given address | ||
| Connect(context.Context, ma.Multiaddr) error |
There was a problem hiding this comment.
Should this take a peerstore.PeerInfo? It should probably take either that or a peer.ID
I'm actually doing this right now, for that reason. |
81d427d to
d0e7d1a
Compare
b4bfdd8 to
71d539b
Compare
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
71d539b to
6fa2ab0
Compare
|
(rebased) |
| Address() ma.Multiaddr | ||
|
|
||
| // Direction returns which way the connection was established | ||
| Direction() net.Direction |
There was a problem hiding this comment.
Any reason to have this over Stat? The nice thing about Stat is that it's extensible.
There was a problem hiding this comment.
Assuming you mean go-libp2p-net.Stat - ipfs swarm peers doesn't return info from the Extra map, so we would have to expose it somehow for a good implementation in go-ipfs-api.
There was a problem hiding this comment.
Got it. Yeah, that may be tricky as it could have arbitrary data. We can punt on that.
core/coreapi/interface/swarm.go
Outdated
| Direction() net.Direction | ||
|
|
||
| // Latency returns last known round trip time to the peer | ||
| Latency(context.Context) (time.Duration, error) |
There was a problem hiding this comment.
As with the pubsub API, we need to consider what this parameter signals. Personally, when I see something taking a context, I immediately think that it does some kind of network operation.
core/coreapi/swarm.go
Outdated
|
|
||
| swrm, ok := api.node.PeerHost.Network().(*swarm.Swarm) | ||
| if !ok { | ||
| return fmt.Errorf("peerhost network was not swarm") |
There was a problem hiding this comment.
If possible, can we just make this optional? That is, clear the backoff iff we have a swarm but don't error out.
There was a problem hiding this comment.
(I realize this probably isn't what we do now but, well, no time better than now)
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
Based on #4802
Replaces #4774
TODOs: