APIE-571 - update for default compute pool #3193
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
Analysis Details3 Issues
Coverage and Duplications
Project ID: cli |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Flink default compute pools, allowing users to submit statements without explicitly specifying a compute pool. The backend will provision a "default" compute pool managed on the user's behalf.
Key Changes:
- Added
--default-poolflag to compute pool create/update commands - Added new
compute-pool-configcommand group for managing default pool settings - Modified statement creation and shell commands to accept
--cloudand--regionflags as alternatives to--compute-pool
Reviewed Changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/flink/command.go | Added compute-pool-config command registration and extraneous comment |
| internal/flink/command_compute_pool.go | Added DefaultPool field to computePoolOut struct |
| internal/flink/command_compute_pool_*.go | Updated create/update/describe/list commands to handle default pool flag |
| internal/flink/command_compute_pool_config*.go | New files implementing compute pool config describe/update commands |
| internal/flink/command_statement*.go | Modified to support creating statements without compute pool using cloud/region |
| internal/flink/command_shell.go | Updated shell command to support default compute pool workflow |
| pkg/ccloudv2/flink.go | Added API methods for compute pool config operations |
| test/test-server/*.go | Updated test handlers to support default pool and compute pool config |
| test/flink_test.go | Added integration tests for default pool functionality |
| test/fixtures/output/flink/** | Updated golden files to include Default Pool column/field |
| go.mod | Updated SDK dependencies to support new API features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
internal/flink/command.go
Outdated
| return cmd | ||
| } | ||
|
|
||
| // here |
There was a problem hiding this comment.
Remove the extraneous comment '// here' which appears to be leftover debugging or placeholder text.
| // here |
cqin-confluent
left a comment
cqin-confluent
left a comment
There was a problem hiding this comment.
Looks great overall! Left a few comments
| return err | ||
| } | ||
| } else { | ||
| client, err = c.GetFlinkGatewayClient(false) |
There was a problem hiding this comment.
Same here. Did we verify the updated logic is working as expected?
There was a problem hiding this comment.
yes this is well tested.
|
Release Notes
Breaking Changes
New Features
Bug Fixes
Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
This will allow user to submit statement without an explicitly specified compute pool (CP). Instead, backend will provision a “default” compute pool (DCP) that is created and managed on the user’s behalf.
Blast Radius
Flink statement create and flink compute pool commands could be affected for existing as well as new behaviour.
References
https://confluentinc.atlassian.net/browse/APIE-571
Test & Review
Added extensive integration tests
Manual Testing:
Statement create without compute pool:
Starting flink shell without compute pool:
Without this change:
CLI Flag --default-pool
Default Compute Pool and Default Max CFU: