lbaas: l7policies: don't omit invert: bool=false - #1314

Build failed.

• gophercloud-unittest : FAILURE in 4m 37s
• gophercloud-acceptance-test : SUCCESS in 1h 18m 06s

@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 admin_state_up values.

As far as I understood there could be only two cases:

• using the pointer AdminStateUp *bool json:"admin_state_up,omitempty"``
• using the pure bool AdminStateUp bool json:"admin_state_up"``

$ 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 Name or Description fields)

@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.

• gophercloud-unittest : FAILURE in 4m 11s
• gophercloud-acceptance-test : FAILURE in 41m 46s

@kayrus If you remove omitempty from these fields, then they will always send false, even if the field wasn't specified. This is why Travis is reporting an error.

Instead of setting omitempty, you will need to change the field from bool to *bool. If you grep the Gophercloud repo for *bool, you'll see a lot of examples to go off of.

@jtopjian I'm aware about the pointer. I didn't want to break the compatibility. That is why I created a #1332 issue to discuss changes.

Sure - I'll jump over there :)

Coverage remained the same at 79.621% when pulling 15d6cd8 on kayrus:kayrus/no-revert-omitempty into fac2636 on gophercloud:master.

Build failed.

• gophercloud-unittest : SUCCESS in 3m 23s
• gophercloud-acceptance-test : FAILURE in 51m 43s

recheck

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 25s
• gophercloud-acceptance-test : SUCCESS in 1h 13m 43s

Good news. I did review all the grepped results and surprisingly it appeared that the only one bool related issue was in the l7policies/requests.go.

@kayrus Nice - thank you for reviewing everything :)

Is this ready to review/merge?

@jtopjian yes.