feat: attempt translate filecoin SP-like provider addresses with heyfil #441
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## rvagg/--provider-http-shortcuts #441 +/- ##
===================================================================
- Coverage 76.40% 75.91% -0.49%
===================================================================
Files 85 86 +1
Lines 6458 6586 +128
===================================================================
+ Hits 4934 5000 +66
- Misses 1259 1305 +46
- Partials 265 281 +16
|
pkg/heyfil/heyfil.go
Outdated
| if isPeerId(s) { | ||
| res, err := h.translatePeerId(s) | ||
| if err != nil { | ||
| logger.Debugw("failed to translate peer id", "input", s, "err", err) | ||
| return "", err | ||
| } | ||
| return res, nil | ||
| } |
There was a problem hiding this comment.
not sure if this is net good or net bad - it'd be better to have this part use the delegated peer routing more generally, since many peers may not be fil specific. cc @masih
There was a problem hiding this comment.
Keep in mind that current behaviour would reject peer IDs entirely. They must be fully formed multiaddrs with a peer ID on the end. So this is strictly additive in that it'll now accept at least some subset of resolvable peer IDs, it's just that subset that heyfil can tell us.
We could just stick to f0 addresses for now to keep it simpler, I just figure that as long as there's an easy way to reject fewer types of input then we may as well try.
| if len(addrs) == 0 { | ||
| return "", fmt.Errorf("expected addr_info.Addrs to be non-empty") | ||
| } | ||
| addr, ok := addrs[0].(string) |
There was a problem hiding this comment.
This is probably flawed. It expects at least one Addr and uses the first. It could potentially use multiple addresses and just treat them as separate providers to try. I'm just not sure what's typically recorded here by SPs.
disabled in code, but can still be done in the heyfil package
rvagg
force-pushed
the
rvagg/--provider-heyfil
branch
from
14e022a to
8ad02d4
Compare
One for you to review @willscott.
This ends up being a complete noop if none of the provided addresses look like they can be translated. So for current usage this is entirely transparent, but as soon as someone provides an
f0\d+or just a plain peer ID, it'll try and use heyfil to fill in the blank.I don't have a good example handy of a specific bit of content and a working SP, but this one should work for some known content that the SP claims to be storing, using both forms: