Skip to content

Conversation

@jiangzho
Copy link
Contributor

What changes were proposed in this pull request?

This PR updates example hot property override in values.yaml / operatorConfiguration.dynamicConfig.data, make it including key value pairs of strings

Why are the changes needed?

Does this PR introduce any user-facing change?

Number value in example may cause helm failure

Error: INSTALLATION FAILED: ConfigMap in version "v1" cannot be handled as a ConfigMap: json: cannot unmarshal number into Go struct field ConfigMap.data of type string

Also, in fact Spark Operator consumes all property override as string values, it would parse the actual types accordingly.

How was this patch tested?

value parsing covered by existing CI, helm installation tested via minikube setup and custom value override

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the BUILD label Aug 29, 2024
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

This seems to be a duplication of

cc @TQJADE too

dongjoon-hyun pushed a commit that referenced this pull request Aug 31, 2024
### What changes were proposed in this pull request?
Add one more e2e test to validate the hot properties reload.

### Why are the changes needed?
1. It will verify Spark Operator reload the properties from dynamic config map
2. It will verify the watched namespace worked as expected

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
Test from personal GitHub workflow Actions

### Was this patch authored or co-authored using generative AI tooling?
no

Closes #112, #114

Closes #115 from TQJADE/hot-properties-test.

Authored-by: Qi Tan <qi_tan@apple.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun
Copy link
Member

This is superceded by #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants