feat: Sequential and Parallel Routers #58
Conversation
ajnavarro
force-pushed
the
feature/paralel-sequential-routers
branch
from
ae267ce
to
665ea8f
Compare
compsequential.go
Outdated
| case <-ctx.Done(): | ||
| return |
There was a problem hiding this comment.
If the context is cancelled this cancels the current router but then it continues w/ the rest of the routers, we probably ought to short-circuit the whole loop at this point to avoid kicking off a bunch of wasteful queries.
There was a problem hiding this comment.
hmm, that's tricky, because that context is the specific router context with the timeout, so maybe the next routers still have execution time.
ajnavarro
force-pushed
the
feature/paralel-sequential-routers
branch
from
d799146
to
f38160d
Compare
ajnavarro
changed the title
ajnavarro
force-pushed
the
feature/paralel-sequential-routers
branch
2 times, most recently
from
dccecff
to
8fd91c4
Compare
| type ProvideManyRouter interface { | ||
| ProvideMany(ctx context.Context, keys []multihash.Multihash) error | ||
| Ready() bool | ||
| } |
There was a problem hiding this comment.
This interface is duplicated in multiple places and is ultimately what we need to consume a router in Kubo, right? Is there a way to move this somewhere authoritative? Should this be in libp2p? Also does this need to be exported?
There was a problem hiding this comment.
I think it should be where routing.Routing interface is living. It is public to at least have one source of truth that can be used anywhere.
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
ajnavarro
force-pushed
the
feature/paralel-sequential-routers
branch
from
8fd91c4
to
0c84fe9
Compare
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
| !errors.Is(err, routing.ErrNotFound) && | ||
| !router.IgnoreError { | ||
| cancel() | ||
| break |
There was a problem hiding this comment.
Do you think it makes sense to log these attempts and errors at info/debug level so that someone can debug what's happening? (if so, we can follow up with this in a separate PR)
There was a problem hiding this comment.
yep, it would make sense
|
It closes #56