Octavia l7 rule support - part 5(final): update #936
Conversation
For gophercloud#832 Octavia l7 rule PUT API implementation: - https://github.com/openstack/octavia/blob/69a45c254be854dbdbff946dbe2564711cdecec3/octavia/api/v2/controllers/l7rule.py#L189
|
Build succeeded.
|
|
@jtopjian @jrperritt the last one patch for review |
|
@lingxiankong It looks like I reviewed #924 wondering how we missed that one earlier and saw that you linked to the I think the addition of this field in both The field definition would be the same for both AdminStateUp *bool `json:"admin_state_up,omitempty"`Other than that, this looks good to me :) |
|
@lingxiankong Again, commented too early - sorry. It looks like It's best to make a separate PR at this point. You can use #832 as the referenced issue -- no need to make a new issue. The PR will simply include the missing |
There was a problem hiding this comment.
@lingxiankong One change to make. Please make the same update to the Update func, too.
But as per the other comments, let's still defer the addition of the AdminStateUp fields to a separate PR.
|
|
||
| // UpdateRule allows Rule to be updated. | ||
| func UpdateRule(c *gophercloud.ServiceClient, policyID string, ruleID string, opts UpdateRuleOptsBuilder) (r UpdateRuleResult) { | ||
| b, _ := opts.ToRuleUpdateMap() |
There was a problem hiding this comment.
I was about to merge this and caught this. I see it's also done with the Update function which I didn't catch earlier either.
Granted it's really hard to trigger an error with this, we should still check for an error regardless.
b, err := opts.ToRuleUpdateMap()
if err != nil {
r.Err = err
return
}
|
Build succeeded.
|
|
LGTM - thank you for the update :) |
Vendor newer terraform-plugin-sdk.
For #832
Octavia l7 rule PUT API implementation: