lbaas: l7policies: don't omit invert: bool=false#1314
lbaas: l7policies: don't omit invert: bool=false#1314jtopjian merged 1 commit intogophercloud:masterfrom
Conversation
|
Build failed.
|
|
@kayrus Thank you for submitting this, but the comment I made previously still applies here: #1313 (comment) |
|
@jtopjian, I'll take care about your suggestions soon. I've noticed that there is a lot of inconsistencies like this. E.g. for the As far as I understood there could be only two cases:
$ grep -r ' bool `json:' | grep omit
compute/v2/extensions/quotasets/requests.go: Force bool `json:"force,omitempty"`
networking/v2/extensions/trunks/results.go: AdminStateUp bool `json:"admin_state_up,omitempty"`
networking/v2/extensions/subnetpools/requests.go: Shared bool `json:"shared,omitempty"`
networking/v2/extensions/subnetpools/requests.go: IsDefault bool `json:"is_default,omitempty"`
sharedfilesystems/v2/shares/results.go: IsPublic bool `json:"is_public,omitempty"`
blockstorage/v3/volumes/requests.go: Multiattach bool `json:"multiattach,omitempty"`
blockstorage/extensions/quotasets/requests.go: Force bool `json:"force,omitempty"`
blockstorage/extensions/volumeactions/requests.go: Force bool `json:"force,omitempty"`
loadbalancer/v2/l7policies/requests.go: Invert bool `json:"invert,omitempty"`
loadbalancer/v2/l7policies/requests.go: Invert bool `json:"invert,omitempty"`$ grep -r ' \*bool `json:' | grep -v omit
networking/v2/extensions/vpnaas/services/requests.go: AdminStateUp *bool `json:"admin_state_up"`Does it make sense to fix that as well? Here are more details golang/go#13284 |
|
same behavior with empty strings (e.g |
|
@kayrus It's really best to open an issue to discuss these changes rather than first submit a Pull Request for one change and then discuss further changes in the same Pull Request. As for the validity of these changes, it's possible there are some parts of the code that need updated to use pointers and it's possible that the current code is correct and the various OpenStack API services are not consistent. We will need to see references to actual Python code in order to determine this. All of this (and more!) is covered in our contributor tutorial 😄 |
|
recheck |
|
Build failed.
|
|
@kayrus If you remove Instead of setting |
|
Sure - I'll jump over there :) |
46b0631 to
5b26234
Compare
5b26234 to
ae60630
Compare
ae60630 to
15d6cd8
Compare
|
Build failed.
|
|
recheck |
|
Build succeeded.
|
|
Good news. I did review all the grepped results and surprisingly it appeared that the only one bool related issue was in the |
|
@kayrus Nice - thank you for reviewing everything :) Is this ready to review/merge? |
|
@jtopjian yes. |
When the rule was initially created with
invert=true, it is not possible to update it toinvert=false, because json engine omitsfalsevalue.