Skip to content
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

Fix issue where pool settings gets overwritten when adding a monitor #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kokhang
Copy link

@kokhang kokhang commented Jul 7, 2016

Fixing it by getting a reference of the pool and adding the monitor to it.
#13

@aburnett
Copy link
Collaborator

aburnett commented Jul 8, 2016

Gah, this happens b/c Go doesn't have nullable primitives so we can't tell when to ignore sending a value. Plus the build in json marshaller is fairly limited is customization w/o handling the marshaling for all objects by hand.

It's a bummer to have to make an extra round trip call. To get around that we could have an object just for this call which only sends in the monitor name. Then only that field would get set on the pool and leave the others intact.

Something like:

type PoolMonitor struct {
  Monitor string `json:"monitor,omitempty"`
}

Then send that in as the config instead of a full Pool. I would vote for avoiding an extra round trip call where possible.

@zetaab
Copy link
Contributor

zetaab commented Mar 13, 2018

I would not accept this solution because using PATCH http method this can be solved correctly. Now there is possibility that someone else will update configuration between these two requests and then changes will be lost

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants