-
Notifications
You must be signed in to change notification settings - Fork 55
SIP-4: DWN Routes #113
SIP-4: DWN Routes #113
Conversation
Codecov Report
@@ Coverage Diff @@
## main #113 +/- ##
==========================================
- Coverage 27.76% 27.69% -0.07%
==========================================
Files 16 17 +1
Lines 1059 1123 +64
==========================================
+ Hits 294 311 +17
- Misses 723 769 +46
- Partials 42 43 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
pkg/server/framework/request.go
Outdated
|
||
resp, err := http.Post(endpoint, "application/json", bytes.NewBuffer(jsonData)) | ||
|
||
if err != nil { |
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.
here too - what kind of error would be returned to the client and logged to us?
pkg/server/router/dwn.go
Outdated
return framework.NewRequestError(errors.Wrap(err, errMsg), http.StatusBadRequest) | ||
} | ||
|
||
status := dwnResp.StatusCode >= 200 && dwnResp.StatusCode < 300 |
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.
status is a binary value?
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.
I'll fix this
|
||
if err != nil { | ||
errMsg := "could not publish manifest to dwn" | ||
if err != nil || &publishManifestResponse.Manifest == nil { |
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.
consider an IsEmpty
method on publishManifestResponse
New DWN route for publishing a manifest, with dwn config for dwn endpoint
Input:
{{baseUrl}}/v1/dwn/manifests
Output