-
Notifications
You must be signed in to change notification settings - Fork 455
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
[dbnode] Add placement set handler for perform set operations #2108
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2108 +/- ##
======================================
Coverage 32.7% 32.7%
======================================
Files 4 4
Lines 336 336
======================================
Hits 110 110
Misses 216 216
Partials 10 10
Continue to review full report at Codecov.
|
TODO tests |
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.
LGTM pending tests and 404 on failed get
"time" | ||
|
||
"github.com/m3db/m3/src/cluster/placement" | ||
|
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.
Nit: extra space?
curPlacement, err := service.Placement() | ||
if err != nil { | ||
logger.Error("unable to get current placement", zap.Error(err)) | ||
xhttp.Error(w, err, http.StatusInternalServerError) |
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.
Let's make this StatusNotFound
? Thinking from the operator perspective, there are times where we have to type check a 404 from Coordinator API.
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.
Sounds good, I added this.
What this PR does / why we need it:
Allows us to do goal state based placement updates, must be used with care!!
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: