-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: adds http DAG api #1930
feat: adds http DAG api #1930
Conversation
946f455 to
6111234
Compare
alanshaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ 🚀 Thanks for this!
src/http/api/resources/dag.js
Outdated
|
|
||
| if (key.codec === 'dag-pb' && result.value) { | ||
| if (typeof result.value.toJSON === 'function') { | ||
| result.value = result.value.toJSON() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if this is required? JSON.stringify will call this automatically...which is what I assume happens after you pass this to h.response 😝.
I also assume that because we're not setting the Content-Type header explicitly that Hapi will infer text/plain from the string you create here and pass to h.response, instead of application/json (which is what we want).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the HTTP API docs /dag/get should return a text/plain response body, which seems like an odd decision.
a499fc9 to
976e955
Compare
alanshaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, just waiting for CI 👍
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
98414f1 to
ec3b44f
Compare
No description provided.