Add RoutingDHT that implements the routing.Routing interface #947
Conversation
| @@ -383,7 +383,7 @@ func (c *Coordinator) QueryMessage(ctx context.Context, msg *pb.Message, fn coor | |||
| ctx, span := c.tele.Tracer.Start(ctx, "Coordinator.QueryMessage") | |||
| defer span.End() | |||
| if msg == nil { | |||
| return coordt.QueryStats{}, fmt.Errorf("no message supplied for query") | |||
| return nil, coordt.QueryStats{}, fmt.Errorf("no message supplied for query") | |||
There was a problem hiding this comment.
What's this change? Was it not building before? Or is it a merge conflict?
| @@ -61,7 +61,7 @@ func TestDHT_FindPeer_happy_path(t *testing.T) { | |||
| d4 := top.AddServer(nil) | |||
| top.ConnectChain(ctx, d1, d2, d3, d4) | |||
|
|
|||
| addrInfo, err := d1.FindPeer(ctx, d4.host.ID()) | |||
| addrInfo, err := NewRouting(d1).FindPeer(ctx, d4.host.ID()) | |||
There was a problem hiding this comment.
Maybe you can wrap the dhts when you create them above
There was a problem hiding this comment.
Would you let the topology operate on Routing DHTs?
|
We should debate whether this actually belongs in Boxo, for example here |
|
Just thinking about how best to decouple the Routing adapter Can we avoid accessing the Same for accessing These aren't essential for this PR but something to think about as we evolve the usability of this implementation. |
|
As per #954 this needs to be applied to zikade |
|
Closing as Reference: probe-lab/zikade#78 |
Not sure I like that. It would also be good to point to the concerns that people have with the routing.Routing interface. This makes the tests look a little less nice unless we let the topology solely operate on
RoutingDHT's.fixes #929