-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support json HTTP announce #1296
Conversation
Codecov ReportBase: 54.66% // Head: 54.56% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #1296 +/- ##
==========================================
- Coverage 54.66% 54.56% -0.11%
==========================================
Files 152 152
Lines 15657 15698 +41
==========================================
+ Hits 8559 8565 +6
- Misses 6126 6157 +31
- Partials 972 976 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
server/ingest/http/server.go
Outdated
buff := new(bytes.Buffer) | ||
err = am.MarshalCBOR(buff) | ||
if err != nil { | ||
httpserver.HandleError(w, err, "announce") | ||
return | ||
} | ||
err = s.ingestHandler.Announce(buff) |
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 think it would be better to pass the decoded message into Announce
instead of re-encoding it.
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.
Making changes as per the suggestion
Context
Fixes #1093
Proposed Changes
Instead of only accepting CBOR:
Read the request Content-Type header
if it is JSON,
parse the body as JSON
convert to CBOR
and pass onto the indexer internal handlers
Otherwise, fallback on CBOR decoding as it is working currently.
Tests
TestJSONSend
Revert Strategy
Change is safe to revert.