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

drop extension / update crd #147

Merged
merged 19 commits into from
Mar 6, 2023
Merged

drop extension / update crd #147

merged 19 commits into from
Mar 6, 2023

Conversation

ChuckHend
Copy link
Collaborator

@ChuckHend ChuckHend commented Mar 6, 2023

Operator creates/drops postgres extensions.

Updated the CRD for extensions to be a list of objects, instead of a list of strings. Each object has as attribute: enabled. Operator will use that attribute to switch its behavior.

@ChuckHend ChuckHend changed the title Operator extensions drop extension / update crd Mar 6, 2023
@ChuckHend ChuckHend marked this pull request as ready for review March 6, 2023 14:22
Comment on lines +97 to +106
impl Default for Extension {
fn default() -> Self {
Extension {
name: "pg_stat_statements".to_owned(),
version: "1.9".to_owned(),
enabled: true,
schema: "postgres".to_owned(),
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
impl Default for Extension {
fn default() -> Self {
Extension {
name: "pg_stat_statements".to_owned(),
version: "1.9".to_owned(),
enabled: true,
schema: "postgres".to_owned(),
}
}
}

I think the default should be an empty vec.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the basis for this being empty? This is the default method and it is explicit. You only gets used if you're calling ::Default().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's an example case for using this? I could see us using it as a default CRD value if we wanted this extension enabled whenever the user provides nothing in spec.extensions (like you had previously configured), but we want that to be empty in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use ::Default() a lot in tests or wherever I want to construct a default set of values for struct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 seems reasonable!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default_extensions is an empty vec regardless of implementing Default on the Extension

coredb-operator/src/defaults.rs Outdated Show resolved Hide resolved
coredb-operator/src/extensions.rs Show resolved Hide resolved
coredb-operator/src/extensions.rs Outdated Show resolved Hide resolved
ChuckHend and others added 3 commits March 6, 2023 09:01
Co-authored-by: Ian Stanton <ian@coredb.io>
Co-authored-by: Ian Stanton <ian@coredb.io>
@ChuckHend ChuckHend merged commit 75cb4e1 into main Mar 6, 2023
@ChuckHend ChuckHend deleted the operator-extensions branch March 6, 2023 16:28
sjmiller609 pushed a commit that referenced this pull request Dec 5, 2023
Co-authored-by: Ian Stanton <ian@coredb.io>
sjmiller609 added a commit that referenced this pull request Dec 5, 2023
* Enable CNPG by default

* pgmq enabled on initial creation

* adjust tests

* Fix tests

* fmt

* Fix race condition
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.

None yet

3 participants