Expose new API fields in CLI (#289)#301
Conversation
Wire up the four groups of API surface landed in the drift fixes (#298/#299/#300) to user-facing CLI commands: 1. ClickPipe object-storage ingestion control: `--skip-initial-load` and `--start-after` (continuous/SQS pipes only). `--skip-initial-load` requires `--queue-url`; `--start-after` conflicts with `--skip-initial-load`. 2. Service horizontal autoscaling on `service create`/`service scale`: `--min-replicas`/`--max-replicas`/`--autoscaling-mode`. The horizontal pair is mutually exclusive with the vertical flags. Infers `horizontal` when the replica pair is present without an explicit mode, and rejects min-without-max (and vice versa) before any network call. 3. ClickPipe MySQL `--server-id` for CDC replication. 4. ClickPipe schema discovery (beta): new `clickpipe schema-discover` command with `kafka`/`kinesis` subcommands that POST to `/clickpipes/schemaDiscovery` and render the inferred fields. Source connection flags are shared with the `create` subcommands via `KafkaSourceFields`/`KinesisSourceFields` flatten structs, so there is no `service_id` positional clash and no destination fields. Adds `AutoscalingMode::VALUES` to the API library (drift-guarded by the analyzer enum-values check) and a `click_pipe_schema_discovery` wrapper on `CloudClient`. Tests: - bin: build_*_request unit tests (minimal + maximal) for horizontal autoscaling on create and scale, plus resolve_horizontal_autoscaling edge cases; Cli::try_parse_from coverage for all new flags. - wiremock: object-storage skipInitialLoad/startAfter, MySQL serverId, and schema-discovery POST body shape for kafka + kinesis.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| access_key, | ||
| use_enhanced_fan_out: if args.enhanced_fan_out { Some(true) } else { None }, | ||
| iterator_type: parse_enum(&args.iterator_type)?, | ||
| timestamp: args.iterator_timestamp.map(|t| t as i64), |
There was a problem hiding this comment.
🟡 Medium cloud/commands.rs:1253
build_kinesis_source converts iterator_timestamp from u64 to i64 with t as i64, so values above i64::MAX silently wrap to negative. Passing --iterator-timestamp 18446744073709551615 sends timestamp: -1 to the API instead of returning a CLI error. Use i64::try_from(t) and return an error on overflow.
- timestamp: args.iterator_timestamp.map(|t| t as i64),
+ timestamp: args.iterator_timestamp.map(|t| i64::try_from(t)).transpose()?,🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhousectl/src/cloud/commands.rs around line 1253:
`build_kinesis_source` converts `iterator_timestamp` from `u64` to `i64` with `t as i64`, so values above `i64::MAX` silently wrap to negative. Passing `--iterator-timestamp 18446744073709551615` sends `timestamp: -1` to the API instead of returning a CLI error. Use `i64::try_from(t)` and return an error on overflow.
| ClickPipeCommands::Stop { .. } => true, | ||
| ClickPipeCommands::Resync { .. } => true, | ||
| ClickPipeCommands::Scale { .. } => true, | ||
| ClickPipeCommands::SchemaDiscover { .. } => false, |
There was a problem hiding this comment.
🟠 High cloud/cli.rs:358
ClickPipeCommands::SchemaDiscover returns false from is_write_command, so when a user authenticates with OAuth/Bearer the early auth guard in run_cloud is skipped and the request is sent to the API, which rejects it with a 403 instead of showing the intended AuthRequired guidance. The schema-discovery endpoint requires API-key authentication, so this variant must return true like the other write commands.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhousectl/src/cloud/cli.rs around line 358:
`ClickPipeCommands::SchemaDiscover` returns `false` from `is_write_command`, so when a user authenticates with OAuth/Bearer the early auth guard in `run_cloud` is skipped and the request is sent to the API, which rejects it with a 403 instead of showing the intended `AuthRequired` guidance. The schema-discovery endpoint requires API-key authentication, so this variant must return `true` like the other write commands.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5d42a31. Configure here.
| autoscaling_mode, | ||
| min_replicas: Some(f64::from(min)), | ||
| max_replicas: Some(f64::from(max)), | ||
| }) |
There was a problem hiding this comment.
Equal replicas infer horizontal
High Severity
The resolve_horizontal_autoscaling helper can send invalid combinations of autoscaling mode and replica counts to the API. It incorrectly infers horizontal mode when --min-replicas and --max-replicas are equal (and mode is omitted), and allows vertical mode with unequal min/max replicas. This leads to API rejections or unintended autoscaling behavior.
Reviewed by Cursor Bugbot for commit 5d42a31. Configure here.
| autoscaling_mode, | ||
| min_replicas: None, | ||
| max_replicas: None, | ||
| }) |
There was a problem hiding this comment.
Vertical mode with replica band
Medium Severity
The resolve_horizontal_autoscaling function in commands.rs can generate invalid API requests for service creation and scaling. It allows --autoscaling-mode vertical with unequal --min-replicas/--max-replicas, which the API expects for horizontal scaling. It also permits --autoscaling-mode horizontal without the required replica bounds, potentially causing API errors.
Reviewed by Cursor Bugbot for commit 5d42a31. Configure here.
| /// Object key to start continuous ingestion after. Mutually exclusive with | ||
| /// --skip-initial-load (the API rejects both being set). | ||
| #[arg(long, conflicts_with = "skip_initial_load")] | ||
| pub start_after: Option<String>, |
There was a problem hiding this comment.
Start-after without queue URL
Medium Severity
The clickpipe create object-storage command allows --start-after to be specified without --queue-url. This is an invalid combination for the API, which expects startAfter only for queue-based continuous ingestion, potentially causing API request failures.
Reviewed by Cursor Bugbot for commit 5d42a31. Configure here.
| opts: ServiceScaleOptions, | ||
| json: bool, | ||
| ) -> Result<(), Box<dyn std::error::Error>> { | ||
| let request = build_service_scale_request(&opts)?; |
There was a problem hiding this comment.
Scale output ignores horizontal
Low Severity
After a successful horizontal service scale, the non-JSON success text still prints only vertical fields (min_replica_memory_gb, max_replica_memory_gb, num_replicas) and omits autoscaling_mode, min_replicas, and max_replicas returned on Service.
Reviewed by Cursor Bugbot for commit 5d42a31. Configure here.
1 participant
Summary
Exposes the four groups of API surface landed in the OpenAPI drift fixes (#298 / #299 / #300) as user-facing CLI commands, per the plan in #289.
Closes #289.
Changes
1. ClickPipe object-storage ingestion control
--skip-initial-load: skip the initial snapshot load, only ingest new objects (continuous/SQS pipes only; requires--queue-url).--start-after: resume ingestion after a specific object key (conflicts with--skip-initial-load).skipInitialLoad/startAfteron the object-storage source body.2. Service horizontal autoscaling (
service create/service scale)--min-replicas/--max-replicas/--autoscaling-mode.--num-replicas/--min-replica-memory-gb/--max-replica-memory-gb) via clapconflicts_with_all.horizontalwhen the replica pair is present without an explicit--autoscaling-mode.--min-replicaswithout--max-replicas(and vice versa) with a clear error before any network call.resolve_horizontal_autoscaling→HorizontalAutoscalingstruct (used by both create and scale viabuild_create_service_request/build_service_scale_request).3. ClickPipe MySQL
--server-idserverIdon the MySQL source body.4. ClickPipe schema discovery (beta)
clickpipe schema-discover <SERVICE_ID> <kafka|kinesis>command./clickpipes/schemaDiscoveryand renders the inferred fields (table by default, JSON with--json/ agent detection).createsubcommands viaKafkaSourceFields/KinesisSourceFields#[command(flatten)]structs — noservice_idpositional clash, no destination (--name/--database/--table/--column) flags needed.API library
AutoscalingMode::VALUESconst (drift-guarded by the analyzer's existing enum-values check).click_pipe_schema_discoverywrapper onCloudClient.Tests
build_create_service_request/build_service_scale_requestunit tests (minimal + maximal) for horizontal autoscaling;resolve_horizontal_autoscalingedge cases (explicit vertical with no replicas, min-without-max rejection, invalid mode);Cli::try_parse_fromcoverage for all new flags (object-storage ingestion, MySQL server-id, autoscaling on create/scale, schema-discover kafka/kinesis).skipInitialLoad/startAfteron the wire (present + absent); MySQLserverId(present + absent); schema-discovery POST body shape for kafka + kinesis.cargo clippy -p clickhouse-cloud-api -p clickhouse-openapi-analyzer --all-targets -- -D warningsclean.python3 scripts/check-openapi-drift.py --dry-run→ 0 actionable drift.cargo check --workspace --all-featuresclean.Docs
service create/scaleexamples; object-storage continuous ingestion example with--skip-initial-load/--start-after; MySQL--server-idexample; new "Discovering a source schema (beta)" subsection under ClickPipes.