-
Notifications
You must be signed in to change notification settings - Fork 29
Bump cmf-sdk-go v0.0.5 to v0.0.6 and fix breaking changes #3320
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
1c59e6e
adf7963
d18db04
2704dd1
d16c1b8
a182d1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| package flink | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
||
| cmfsdk "github.com/confluentinc/cmf-sdk-go/v1" | ||
|
|
||
| "github.com/confluentinc/cli/v4/pkg/config" | ||
| "github.com/confluentinc/cli/v4/pkg/output" | ||
| ) | ||
|
|
||
| type computePoolOut struct { | ||
|
|
@@ -60,6 +63,35 @@ | |
| return c.autocompleteComputePools(cmd, args) | ||
| } | ||
|
|
||
| // extractComputePoolPhase extracts the phase string from the generic status map. | ||
| // ComputePool.Status changed from *ComputePoolStatus to *map[string]map[string]interface{} in SDK v0.0.6. | ||
| // The status is a nested map where phase is at status["phase"]["value"]. | ||
| func extractComputePoolPhase(pool cmfsdk.ComputePool) string { | ||
| if pool.Status == nil { | ||
| return "" | ||
| } | ||
| status := pool.GetStatus() | ||
| if phaseMap, ok := status["phase"]; ok { | ||
| if value, ok := phaseMap["value"].(string); ok { | ||
| return value | ||
| } | ||
| } | ||
| // Fallback: try re-parsing as a simpler type in case the API shape varies. | ||
| raw, err := json.Marshal(pool.Status) | ||
| if err != nil { | ||
| output.ErrPrintf(false, "Warning: failed to marshal compute pool status: %v\n", err) | ||
| return "" | ||
| } | ||
| var flat map[string]string | ||
| if err := json.Unmarshal(raw, &flat); err == nil { | ||
|
Check warning on line 86 in internal/flink/command_compute_pool.go
|
||
| if phase, ok := flat["phase"]; ok { | ||
| return phase | ||
| } | ||
| } | ||
| output.ErrPrintf(false, "Warning: compute pool has status but phase could not be extracted\n") | ||
| return "" | ||
| } | ||
|
Comment on lines
+63
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it make sense to move this utility into the SDK?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is that we don't do human code push in cmf-go-sdk, but if we want to : Adds a custom UnmarshalJSON method on the ComputePool type (in a new file v1/model_compute_pool_custom.go ) seems to be the better fix, which completely removes "custom unmarshaling with json.RawMessage embedding" from cli repo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My only worry about human pushes in the sdk repo is that the code generator is overwriting human written code / utils. |
||
|
|
||
| func convertSdkComputePoolToLocalComputePool(sdkComputePool cmfsdk.ComputePool) LocalComputePool { | ||
| localPool := LocalComputePool{ | ||
| ApiVersion: sdkComputePool.ApiVersion, | ||
|
|
@@ -77,9 +109,9 @@ | |
| }, | ||
| } | ||
|
|
||
| if sdkComputePool.Status != nil { | ||
| if phase := extractComputePoolPhase(sdkComputePool); phase != "" { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we always call this method, also when the CLI runs against older CMF versions? Probably yes? because the response has not changed, only the SDK. |
||
| localPool.Status = &LocalComputePoolStatus{ | ||
| Phase: sdkComputePool.Status.Phase, | ||
| Phase: phase, | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.