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
Add octavia pool member batch update BatchUpdateMembersAdditiveOnly #2863
base: master
Are you sure you want to change the base?
Add octavia pool member batch update BatchUpdateMembersAdditiveOnly #2863
Conversation
73c3358
to
b90b180
Compare
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.
Thank you for submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.
b90b180
to
668c2aa
Compare
members := []map[string]interface{}{} | ||
for _, opt := range opts { | ||
b, err := opt.ToBatchMemberUpdateMap() | ||
if err != nil { | ||
r.Err = err | ||
return | ||
} | ||
members = append(members, b) | ||
} | ||
|
||
b := map[string]interface{}{"members": members} |
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.
Looks like code duplication with BatchUpdateMembers()
that you could minimize by extracting this into a separate function.
668c2aa
to
6f1ff3f
Compare
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.
The code looks good, but I'd like to see acceptance tests showing that the new BatchUpdateMembersAdditiveOnly()
function does what is claims to do. Can we add that?
6f1ff3f
to
38a163e
Compare
38a163e
to
8ff2c2d
Compare
8ff2c2d
to
f7e3160
Compare
Signed-off-by: Adrien Pensart <crunchengine@gmail.com>
f7e3160
to
6d842c6
Compare
acceptance tests added! |
Can we have a review? |
Fixes #[2862]
Related to https://docs.openstack.org/api-ref/load-balancer/v2/#batch-update-members