Skip to content

Commit d0b8d4e

Browse files
committed
Extract cloud runtime dispatch and shared helpers
1 parent 763fa4a commit d0b8d4e

8 files changed

Lines changed: 1374 additions & 1437 deletions

File tree

AGENTS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ The user-facing CLI surface. Contains all logic for local commands, wraps `click
1212

1313
- Cloud handlers go through the `CloudClient` wrapper (`src/cloud/client.rs`), not `clickhouse_cloud_api::Client` directly. The wrapper handles credential precedence, error conversion, and response unwrapping.
1414
- Cloud handlers always support `--json` output unless there is good reason not to. JSON is emitted automatically when `--json` is passed or a coding agent is detected (`is_ai_agent::detect()` via the `json_output()` helper in `main.rs`).
15-
- `CloudError` carries a `kind: CloudErrorKind` (`Auth` for 401/403 and missing credentials, else `Generic`). It maps to `Error::AuthRequired` / `Error::Cloud` in `main.rs`. Dispatched commands exit with `0` on success or use `Error::exit_code()` for failures: `1` error, `3` cancelled, `4` auth required. Clap uses `2` for usage errors.
15+
- `CloudError` carries a `kind: CloudErrorKind` (`Auth` for 401/403 and missing credentials, else `Generic`). It maps to `Error::AuthRequired` / `Error::Cloud` in `cloud::run`. Dispatched commands exit with `0` on success or use `Error::exit_code()` for failures: `1` error, `3` cancelled, `4` auth required. Clap uses `2` for usage errors.
1616

1717
Use `--help` to learn the current command surface.
1818

@@ -22,7 +22,7 @@ The CLI does not need to have 100% coverage of endpoints exposed by the API libr
2222

2323
#### Adding a command
2424

25-
For both local and cloud commands, define the clap variant in the appropriate `cli.rs`, then wire dispatch in `src/main.rs`.
25+
For both local and cloud commands, define the clap variant in the appropriate `cli.rs`, then wire dispatch in the owning runtime module.
2626

2727
**Local subcommand:**
2828

@@ -35,7 +35,7 @@ For both local and cloud commands, define the clap variant in the appropriate `c
3535
1. Make sure `clickhouse-cloud-api` has already been updated to support necessary endpoints & models.
3636
2. Add the variant to the relevant sub-enum in `src/cloud/cli.rs` (or `src/cloud/postgres.rs` for Postgres). Create a new sub-enum if the surface warrants its own grouping.
3737
3. Classify the new variant in `CloudCommands::is_write_command()` in `src/cloud/cli.rs` (Postgres variants go in the equivalent `is_write()` on the Postgres enum). OAuth (Bearer) auth is read-only; write commands require API key auth and we fail fast on OAuth + write. The match has no wildcards, so the compiler will reject a missing arm — but you still need to make the read/write call deliberately, and add a case to both the `is_write_command_read_only_commands` and `is_write_command_destructive_commands` tests.
38-
4. Add the match arm in `run_cloud()` in `src/main.rs`.
38+
4. Add non-Postgres match arms to `cloud::dispatch()` in `src/cloud/mod.rs`. Postgres dispatch belongs in `src/cloud/postgres.rs::run`; if another domain later gains its own runtime dispatcher, have `cloud::dispatch()` delegate to it rather than keeping that domain's match arms centrally.
3939
5. Add a thin wrapper method on `CloudClient` in `src/cloud/client.rs`. It should delegate to `self.api().<lib_method>()`, map errors via `self.convert_error(e)`, and unwrap with `Self::unwrap_response`. Use the library's request/response types here.
4040
6. If the command sends a request body, extract a `build_<name>_request(...)` helper in `src/cloud/commands.rs` that returns the library's request struct. Cover the helper with minimal + maximal unit tests in the `mod tests` block at the bottom of `commands.rs`, asserting directly on library struct fields.
4141
7. Implement the handler in `src/cloud/commands.rs`. For body-sending commands the handler calls the build helper, passes the result through the `CloudClient` wrapper, and prints with the `--json` output pattern. For detail/get views (rendering a single resource), drive human output through `print_human` so it shares serde's behaviour — including deprecated-field hiding — instead of hand-writing `println!` lines:

crates/clickhousectl/src/cli.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
11
use clap::{Args, Parser, Subcommand};
22

3-
pub use crate::cloud::cli::{
4-
ActivityCommands, AuthCommands, BackupCommands, BackupConfigCommands, ClickPipeCommands,
5-
ClickPipeCreateCommands, ClickPipeSettingsCommands, CloudArgs, CloudCommands,
6-
InvitationCommands, KeyCommands, MemberCommands, OrgCommands, PrivateEndpointCommands,
7-
QueryEndpointCommands, ServiceCommands,
8-
};
9-
pub use crate::cloud::postgres::{
10-
CertsCommands as PostgresCertsCommands, ConfigCommands as PostgresConfigCommands,
11-
PostgresCommands, ReadReplicaCommands as PostgresReadReplicaCommands,
12-
};
3+
use crate::cloud::cli::CloudArgs;
134
pub use crate::local::cli::LocalArgs;
145

156
#[derive(Parser)]

crates/clickhousectl/src/cloud/cli.rs

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use chrono::{DateTime, FixedOffset, NaiveDate, NaiveTime};
1+
use crate::cloud::shared::{parse_date_only, parse_datetime, parse_time_only};
22
use clap::builder::PossibleValuesParser;
33
use clap::{Args, Subcommand};
44

@@ -74,32 +74,6 @@ const MONGODB_READ_PREFERENCES: &[&str] = &[
7474
"nearest",
7575
];
7676

77-
fn parse_date_only(value: &str) -> Result<String, String> {
78-
if NaiveDate::parse_from_str(value, "%Y-%m-%d").is_err() {
79-
return Err(format!("invalid date '{}': expected YYYY-MM-DD", value));
80-
}
81-
82-
Ok(value.to_string())
83-
}
84-
85-
pub(super) fn parse_datetime(value: &str) -> Result<String, String> {
86-
if DateTime::<FixedOffset>::parse_from_rfc3339(value).is_err() {
87-
return Err(format!(
88-
"invalid datetime '{}': expected ISO 8601 / RFC 3339",
89-
value
90-
));
91-
}
92-
93-
Ok(value.to_string())
94-
}
95-
96-
fn parse_time_only(value: &str) -> Result<String, String> {
97-
if NaiveTime::parse_from_str(value, "%H:%M").is_err() {
98-
return Err(format!("invalid time '{}': expected HH:MM", value));
99-
}
100-
101-
Ok(value.to_string())
102-
}
10377
#[derive(Subcommand)]
10478
pub enum AuthCommands {
10579
/// Log in to ClickHouse Cloud

crates/clickhousectl/src/cloud/commands.rs

Lines changed: 7 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
use crate::cloud::client::{CloudClient, CloudError};
22
use crate::cloud::credentials;
33
use crate::cloud::output::{ABSENT, or_absent, print_human};
4+
use crate::cloud::shared::{parse_serde_enum, parse_tags, resolve_org_id};
45
use clickhouse_cloud_api::models::{
56
ApiKeyPatchRequest, ApiKeyPatchRequestState, ApiKeyPostRequest, ApiKeyPostRequestState,
67
AutoscalingMode, BackupConfigurationPatchRequest, InstancePrivateEndpointsPatch,
78
InstanceServiceQueryApiEndpointsPostRequest, InstanceTagsPatch, IpAccessListEntry,
89
IpAccessListPatch, OrganizationPatchPrivateEndpoint,
910
OrganizationPatchPrivateEndpointCloudprovider, OrganizationPatchPrivateEndpointRegion,
10-
OrganizationPatchRequest, OrganizationPrivateEndpointsPatch, ResourceTagsV1,
11-
ServicPrivateEndpointePostRequest, Service, ServiceEndpoint, ServiceEndpointChange,
12-
ServiceEndpointChangeProtocol, ServicePasswordPatchRequest, ServicePatchRequest,
13-
ServicePatchRequestReleasechannel, ServicePostRequest, ServicePostRequestCompliancetype,
14-
ServicePostRequestProfile, ServicePostRequestProvider, ServicePostRequestRegion,
15-
ServicePostRequestReleasechannel, ServiceReplicaScalingPatchRequest, ServiceState,
16-
ServiceStatePatchRequestCommand,
11+
OrganizationPatchRequest, OrganizationPrivateEndpointsPatch, ServicPrivateEndpointePostRequest,
12+
Service, ServiceEndpoint, ServiceEndpointChange, ServiceEndpointChangeProtocol,
13+
ServicePasswordPatchRequest, ServicePatchRequest, ServicePatchRequestReleasechannel,
14+
ServicePostRequest, ServicePostRequestCompliancetype, ServicePostRequestProfile,
15+
ServicePostRequestProvider, ServicePostRequestRegion, ServicePostRequestReleasechannel,
16+
ServiceReplicaScalingPatchRequest, ServiceState, ServiceStatePatchRequestCommand,
1717
};
1818
use std::io::{IsTerminal, Write};
1919
use tabled::{Table, Tabled, settings::Style};
@@ -46,17 +46,6 @@ fn first_endpoint(endpoints: Option<&[ServiceEndpoint]>) -> String {
4646
.unwrap_or_else(|| ABSENT.to_string())
4747
}
4848

49-
/// Resolve org ID from explicit arg or auto-detect
50-
pub(super) async fn resolve_org_id(
51-
client: &CloudClient,
52-
org_id: Option<&str>,
53-
) -> Result<String, Box<dyn std::error::Error>> {
54-
match org_id {
55-
Some(id) => Ok(id.to_string()),
56-
None => Ok(client.get_default_org_id().await?),
57-
}
58-
}
59-
6049
/// Resolve a service by name or ID within the given org.
6150
/// Exactly one of `name` or `id` must be provided.
6251
async fn resolve_service(
@@ -88,69 +77,6 @@ async fn resolve_service(
8877
}
8978
}
9079

91-
/// Parse a string into a library enum via serde deserialization, with client-side
92-
/// validation against a known-values list. Library enums have an `Unknown(String)`
93-
/// catch-all that prevents serde from ever failing, so we validate first.
94-
pub(super) fn parse_serde_enum<T: serde::de::DeserializeOwned>(
95-
value: &str,
96-
field: &str,
97-
known_values: &[&str],
98-
) -> Result<T, Box<dyn std::error::Error>> {
99-
if !known_values.contains(&value) {
100-
return Err(format!(
101-
"invalid {}: unknown value '{}', expected one of: {}",
102-
field,
103-
value,
104-
known_values.join(", ")
105-
)
106-
.into());
107-
}
108-
serde_json::from_value(serde_json::Value::String(value.to_string()))
109-
.map_err(|e| format!("invalid {}: {}", field, e).into())
110-
}
111-
112-
pub(super) fn parse_tag(value: &str) -> Result<ResourceTagsV1, Box<dyn std::error::Error>> {
113-
match value.split_once('=') {
114-
Some((key, tag_value)) => {
115-
let key = key.trim();
116-
if key.is_empty() {
117-
Err(format!("invalid tag '{}': tag key cannot be empty", value).into())
118-
} else {
119-
Ok(ResourceTagsV1 {
120-
key: key.to_string(),
121-
value: Some(tag_value.to_string()),
122-
})
123-
}
124-
}
125-
None => {
126-
let key = value.trim();
127-
if key.is_empty() {
128-
Err(format!("invalid tag '{}': tag key cannot be empty", value).into())
129-
} else {
130-
Ok(ResourceTagsV1 {
131-
key: key.to_string(),
132-
value: None,
133-
})
134-
}
135-
}
136-
}
137-
}
138-
139-
pub(super) fn parse_tags(
140-
values: &[String],
141-
) -> Result<Option<Vec<ResourceTagsV1>>, Box<dyn std::error::Error>> {
142-
if values.is_empty() {
143-
Ok(None)
144-
} else {
145-
Ok(Some(
146-
values
147-
.iter()
148-
.map(|value| parse_tag(value))
149-
.collect::<Result<Vec<_>, _>>()?,
150-
))
151-
}
152-
}
153-
15480
fn parse_ip_access_entries(values: &[String]) -> Option<Vec<IpAccessListEntry>> {
15581
(!values.is_empty()).then(|| {
15682
values
@@ -3869,21 +3795,6 @@ mod tests {
38693795
);
38703796
}
38713797

3872-
#[test]
3873-
fn parse_tag_rejects_empty_keys() {
3874-
let err = parse_tag("=value").unwrap_err();
3875-
assert_eq!(
3876-
err.to_string(),
3877-
"invalid tag '=value': tag key cannot be empty"
3878-
);
3879-
3880-
let err = parse_tag(" ").unwrap_err();
3881-
assert_eq!(
3882-
err.to_string(),
3883-
"invalid tag ' ': tag key cannot be empty"
3884-
);
3885-
}
3886-
38873798
#[test]
38883799
fn build_create_service_request_supports_ga_optional_fields() {
38893800
let opts = CreateServiceOptions {

0 commit comments

Comments
 (0)