FEATURE: Add endpoint to resolve peer IDs into IPNS entries. #1546
Conversation
api/endpoints.go
Outdated
| @@ -195,6 +195,8 @@ func get(i *jsonAPIHandler, path string, w http.ResponseWriter, r *http.Request) | |||
| i.GETWalletStatus(w, r) | |||
| case strings.HasPrefix(path, "/ob/ipns"): | |||
| i.GETIPNS(w, r) | |||
| case strings.HasPrefix(path, "/ob/resolveipns"): | |||
| i.ResolveIPNS(w, r) | |||
There was a problem hiding this comment.
Core function ResolveIPNS does not follow pattern of HTTPVERBFunctionName. Nitpick: These two function calls are a little ambiguous to a newcomer. Any way to clarify?
There was a problem hiding this comment.
You're right, I totally overlooked the http verb prefix thing. I'll fix that.
There was a problem hiding this comment.
I'm not sure how to clarify without renaming /ob/ipns, which would not be ideal.
One option would be making it a flag on /ob/ipns, e.g. /ob/ipns?resolve=true
There was a problem hiding this comment.
I can't think of any suggestions to offer either. We can punt if we can't think of anything better. I think the GET param overloads the endpoint with two different behaviors. And really, I would want to change the ipns endpoint to be more specific, but that has repercussions which are annoying. We can leave it as-is for now.
| return | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), time.Second*180) |
There was a problem hiding this comment.
The timeout for this call is 2 minutes. Is this intended? /cc @drwasho
There was a problem hiding this comment.
Yeah I need it to be at least 2 minutes but we could make it configurable if somebody only wanted entries that resolve quickly but clients should use a timeout on their side anyways.
There was a problem hiding this comment.
This is no problem for me, I just want to make sure this was intended.
|
Looks good. |
No description provided.