Repository navigation
Conversation
…switch # Conflicts: # Cargo.lock # Cargo.nix # Cargo.toml # crate-hashes.json # rust/operator-binary/src/controller/build/command.rs # rust/operator-binary/src/crd/mod.rs
Maleware
changed the title
adwk67
moved this from Development: Waiting for Review
to Development: In Review
in Stackable Engineering
| /// [`TrinoRoleGroupConfig`] consumed by the build steps. | ||
| fn into_role_group_config( | ||
| merged: RoleGroup<v1alpha1::TrinoConfig, JavaCommonConfig, v1alpha1::TrinoConfigOverrides>, | ||
| role_group_name: &RoleGroupName, |
There was a problem hiding this comment.
We are passing the role_group_name just for the error, which does not indicate the role. It would be better to catch the error at the call site in validate_role_groups so that both role and role_group can be reported with the error.
| pub fn container_trino_args( | ||
| authentication_config: &TrinoAuthenticationConfig, | ||
| catalogs: &BTreeMap<TrinoCatalogName, CatalogConfig>, | ||
| merged_config: &ValidatedTrinoConfig, |
There was a problem hiding this comment.
We pass this only to use containerdebug. Passing just that field makes the dependency explicit and probably simplifies the tests.
| using OAuth2 client credentials or a bearer token ([#950], [#972]). | ||
| - The logs of the `containerdebug` process are now collected by the Vector agent and forwarded to | ||
| the Vector aggregator ([#970]). | ||
| - Add `containerdebug` config CRD ([#967]). |
There was a problem hiding this comment.
Rather refer to the addition of the config.containerdebug field on roles and role groups. Also, the default interval has (silently) changed from 1m to 30m so this should be mentioned. (ignore that bit: my op-rs branch was out of date)
| #[fragment_attrs(serde(default))] | ||
| pub affinity: StackableAffinity, | ||
|
|
||
| #[fragment_attrs(serde(default))] |
There was a problem hiding this comment.
If this attribute was dropped for some reason, no test would catch it. We also don't test that e.g. interval: 0s produces an error.
| #[case::enabled(ValidatedContainerdebugConfig::Enabled { | ||
| interval: Duration::from_minutes_unchecked(10) | ||
| })] | ||
| #[case::disabled(ValidatedContainerdebugConfig::Disabled)] |
There was a problem hiding this comment.
We should mention in the docs that containerdebug can be turned off.
Description
Spike for stackabletech/operator-rs#1308
part of stackabletech/issues#884
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker