Add HTTP JSONPB request method to client and update callers #163
Conversation
| func (c *client) DoHTTPJSONPBRequest( | ||
| action, url string, | ||
| request proto.Message, | ||
| response proto.Message, |
There was a problem hiding this comment.
Just curious, is the reason to pass the response so that a user can pass a typed one rather than have to type assert on the way back (if signature returned (proto.Message, error)?
There was a problem hiding this comment.
Well the main thing is I need to unmarshal into a specific proto message, unless the caller wants to do the the whole jsonpb Unmarshal thing (which we're trying to avoid them doing). That's why they need to provide up front.
pkg/m3admin/placement/client.go
Outdated
| return nil, errors.New("nil placement fetch") | ||
| } | ||
| p.logger.Debug("placement retreived") | ||
| return m3placement.NewPlacementFromProto(data.Placement) | ||
| p.logger.Info("placement retreived") |
There was a problem hiding this comment.
Nit: I changed this from Info to Debug recently because this happens on every run of the control loop. Since we refresh every N seconds even with no events this means the operator logs could just be "placement retrieved" constantly.
Might be worth keeping this as debug.
There was a problem hiding this comment.
Ah yeah, this was a bad merge, will revert.
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
==========================================
+ Coverage 74.76% 75.13% +0.36%
==========================================
Files 27 28 +1
Lines 1922 1918 -4
==========================================
+ Hits 1437 1441 +4
+ Misses 368 365 -3
+ Partials 117 112 -5Continue to review full report at Codecov.
|
This makes sure that every call site is using the correct JSON marshal/unmarshal and adds helpers so future methods don't have to construct the marshaller themselves and just directly call the
DoHTTPJSONPBRequesthelper.