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

add to postgresql_type.go omitempty annotation #1223

Merged
merged 3 commits into from
Dec 10, 2020

Conversation

Thunderbolt32
Copy link
Contributor

I use the PostgreSQL type to configure a struct instance in golang to create the resource in the k8s cluster.

To create the https://github.com/zalando/postgres-operator/blob/master/manifests/minimal-postgres-manifest.yaml in golang it is necessary to add some more omitempty annotations to unset values.

Closes #1221

@FxKu
Copy link
Member

FxKu commented Nov 18, 2020

Looks similar to #1022
Can you check if there are more sections, that require omitempty?

@Thunderbolt32
Copy link
Contributor Author

In fact it is similar to #1022. As I was still working with v.1.5.0, I had a lot more bugs until I switched to the current master branch.

@FxKu Yes, I'll look at it again. It seems that more pointers would also be useful, because no structure values can be set on maps in Golang. However, I do not know if this would break compatibility. However, I will change it first.

@Thunderbolt32 Thunderbolt32 changed the title add to postgresql_type.go omitempty annotation [WIP] add to postgresql_type.go omitempty annotation Nov 18, 2020
@Thunderbolt32
Copy link
Contributor Author

One thing that would most likely require a code change would be to solve the "not addressable" problem by using struct Pointers (instead of structs) https://stackoverflow.com/a/32751792

However, it only seems to be seriously needed in one case

type PostgresSpec struct {
        ...
	PreparedDatabases     map[string]PreparedDatabase `json:"preparedDatabases,omitempty"`
        ...
}

type PreparedDatabase struct {
	PreparedSchemas map[string]PreparedSchema `json:"schemas,omitempty"`
	...
}

@Thunderbolt32 Thunderbolt32 changed the title [WIP] add to postgresql_type.go omitempty annotation add to postgresql_type.go omitempty annotation Nov 18, 2020
@FxKu
Copy link
Member

FxKu commented Nov 24, 2020

using pointers for the PreparedDatabase structs sound like a story for another PR. I'm also not sure how easy it is to make this backwards compatible.

@Thunderbolt32
Copy link
Contributor Author

I have removed the default value and resolved your review.

@Thunderbolt32
Copy link
Contributor Author

Is the E2E test Flaky? I do not quite understand why the test should fail after this change.

@Thunderbolt32 Thunderbolt32 requested a review from FxKu December 2, 2020 14:24
@FxKu
Copy link
Member

FxKu commented Dec 9, 2020

👍

@FxKu FxKu added this to the 1.6 milestone Dec 9, 2020
@Jan-M
Copy link
Member

Jan-M commented Dec 9, 2020

👍

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.

How to use Cluster manifest in golang?
3 participants