Skip to content

Commit 4a64349

Browse files
committed
feat(ingest): shorthand for delta and kafka, and an honest conflict hint
delta and kafka had single-field selectors and no flag, so both needed raw --selector JSON while sql, iceberg and ducklake had --table. `--table-path` and repeatable `--topic` fill that in; each says in help what the datasource is, since a delta datasource is a storage root rather than a table and a kafka one is a cluster rather than a topic. Neither derives a destination table: a storage path segment is not a table name. No --resource for rest, deliberately. Every other shorthand names its subset in one word; a REST resource is a name plus an endpoint carrying path, method, params, body, which field holds the rows and how to page. A flag could carry only the name, and the failure that makes that dishonest rather than merely partial is that a guessed paginator returns page one and reports success. The error points at --selector and at the field reference instead. The destination-conflict hint contradicted the server message printed directly above it: it always said a continuous ingest owned the table and always prescribed a delete. The holder can now be any family and any type, and the server already reports which -- a one_time holder frees the table when it finishes, so telling someone to delete it tears down a live load that was about to hand the table over. The hint now follows what the server reported, and the empty-reservation case names --dest-table-prefix, the one remedy that touches nothing else. Also shows `probed` in `datasource validate`, so a reader can tell whether the source was contacted rather than inferring it from whether a detail line appeared.
1 parent 4e0b638 commit 4a64349

4 files changed

Lines changed: 414 additions & 27 deletions

File tree

‎src/client/ingest.rs‎

Lines changed: 110 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,7 @@ impl IngestError {
154154
if let IngestError::Http { body, .. } = self {
155155
match util::error_code(body).as_deref() {
156156
Some("destination_table_conflict") => {
157-
let conflicting = util::error_detail(body, "conflicting_ingest_id")
158-
.unwrap_or_else(|| "<ingest-id>".into());
159-
eprintln!(
160-
"{}",
161-
format!(
162-
"A continuous ingest already owns that table. Release it with: \
163-
hotdata ingest delete {conflicting}"
164-
)
165-
.dark_grey()
166-
);
157+
eprintln!("{}", table_conflict_hint(body).dark_grey());
167158
}
168159
// The destination is per-family, and the pair of flags that
169160
// build it is the thing to re-read: a family lands one table
@@ -215,6 +206,46 @@ impl IngestError {
215206
}
216207
}
217208

209+
/// What to DO about a destination-table conflict, chosen from what the service
210+
/// reported about the ingest already holding the reservation.
211+
///
212+
/// The server's message says what happened and which release rule applies; this
213+
/// only names the command. It has to read the reported holder rather than
214+
/// assume one, because the rule covers a saved load of any kind and the kinds
215+
/// release differently — a hint naming one kind unconditionally contradicts the
216+
/// message printed directly above it, and prescribing a delete for a holder
217+
/// that is about to release on its own tears down a live load for nothing.
218+
fn table_conflict_hint(body: &str) -> String {
219+
let conflicting =
220+
util::error_detail(body, "conflicting_ingest_id").unwrap_or_else(|| "<ingest-id>".into());
221+
let releases_itself = matches!(
222+
util::error_detail(body, "conflicting_ingest_type").as_deref(),
223+
Some("one_time") | Some("scheduled")
224+
);
225+
// An empty reservation is the holder taking the source's own table names,
226+
// and it is the one conflict the caller can settle without touching the
227+
// holder: a prefix makes two loads unable to reach the same name. The
228+
// server names the JSON field; this names the flag that fills it.
229+
let unprefixed =
230+
util::error_detail(body, "reserved_table").is_some_and(|reserved| reserved.is_empty());
231+
let mut hint = if unprefixed {
232+
"Give this ingest a --dest-table-prefix so the two cannot land the same table".to_string()
233+
} else if releases_itself {
234+
format!(
235+
"That ingest frees the table when it finishes or is stopped — check it with: \
236+
hotdata ingest show {conflicting}"
237+
)
238+
} else {
239+
format!("Free the table with: hotdata ingest delete {conflicting}")
240+
};
241+
if unprefixed || releases_itself {
242+
hint.push_str(&format!(
243+
", or take the table now with: hotdata ingest delete {conflicting}"
244+
));
245+
}
246+
hint
247+
}
248+
218249
/// Ingest client bound to a workspace + a resolved bearer token.
219250
pub struct IngestClient {
220251
/// `{api_url}/ingest` — api_url already carries the `/v1` suffix.
@@ -650,6 +681,17 @@ pub struct ValidateResponse {
650681
pub normalized_config: Option<serde_json::Value>,
651682
#[serde(default)]
652683
pub discovered: Option<serde_json::Value>,
684+
/// Whether anything actually connected to the source.
685+
///
686+
/// `valid` answers the narrower question of whether the config and
687+
/// credentials PARSE. A family with no read-only probe, and a credential
688+
/// shape no client can be built from, are both `valid: true` having
689+
/// reached nothing — so this is the field that separates a config that
690+
/// checked out from a credential that works, and the reason it is decoded
691+
/// rather than dropped is that `-o json` is this struct serialized.
692+
#[serde(default)]
693+
pub probed: Option<bool>,
694+
/// Why the probe did not run, when it did not.
653695
#[serde(default)]
654696
pub detail: Option<String>,
655697
}
@@ -1145,6 +1187,64 @@ mod tests {
11451187
IngestClient::from_parts(&server.url(), "eyJ.fake.jwt", false, "ws-1")
11461188
}
11471189

1190+
// --- the destination-conflict hint -------------------------------------
1191+
1192+
fn conflict_body(kind: &str, reserved: &str) -> String {
1193+
format!(
1194+
r#"{{"error":{{"code":"destination_table_conflict","message":"...",
1195+
"details":{{"conflicting_ingest_id":"ing_held",
1196+
"conflicting_ingest_type":"{kind}",
1197+
"reserved_table":"{reserved}"}}}}}}"#
1198+
)
1199+
}
1200+
1201+
/// The three cases differ in what the caller can do about them, and the
1202+
/// hint has to differ with them: a holder that never releases on its own,
1203+
/// one that does, and a reservation the caller can step around entirely.
1204+
#[test]
1205+
fn conflict_hint_prescribes_a_delete_only_where_nothing_else_frees_the_table() {
1206+
let hint = table_conflict_hint(&conflict_body("continuous", "orders_raw"));
1207+
assert_eq!(hint, "Free the table with: hotdata ingest delete ing_held");
1208+
}
1209+
1210+
#[test]
1211+
fn conflict_hint_says_a_finishing_holder_frees_the_table_itself() {
1212+
for kind in ["one_time", "scheduled"] {
1213+
let hint = table_conflict_hint(&conflict_body(kind, "orders_raw"));
1214+
assert!(
1215+
hint.starts_with("That ingest frees the table when it finishes or is stopped"),
1216+
"{kind}: {hint}"
1217+
);
1218+
// Delete stays reachable — it is the way to take the table NOW —
1219+
// but as the second option rather than the prescription.
1220+
assert!(hint.ends_with("hotdata ingest delete ing_held"), "{hint}");
1221+
}
1222+
}
1223+
1224+
/// An empty `reserved_table` is the holder taking the source's own table
1225+
/// names. It is the only conflict with a remedy that touches nothing else,
1226+
/// and the remedy is a flag this CLI owns the spelling of.
1227+
#[test]
1228+
fn conflict_hint_offers_a_prefix_when_the_reservation_is_the_raw_names() {
1229+
let hint = table_conflict_hint(&conflict_body("one_time", ""));
1230+
assert!(
1231+
hint.starts_with("Give this ingest a --dest-table-prefix"),
1232+
"{hint}"
1233+
);
1234+
}
1235+
1236+
/// A server that reports no holder type still gets a usable hint rather
1237+
/// than a claim about a type nobody reported.
1238+
#[test]
1239+
fn conflict_hint_falls_back_to_delete_when_the_holder_type_is_absent() {
1240+
let body = r#"{"error":{"code":"destination_table_conflict","message":"...",
1241+
"details":{"conflicting_ingest_id":"ing_held"}}}"#;
1242+
assert_eq!(
1243+
table_conflict_hint(body),
1244+
"Free the table with: hotdata ingest delete ing_held"
1245+
);
1246+
}
1247+
11481248
// --- datasources -------------------------------------------------------
11491249

11501250
#[test]

‎src/commands/datasource.rs‎

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@
3131
//! nothing a script does changed.
3232
//!
3333
//! **Presentation contract:** `validate` persists nothing and is the preflight
34-
//! path; `create` validates again regardless. Secrets are never echoed back by
35-
//! the server, never printed here, and never shown while being typed.
34+
//! path; `create` validates again regardless. `valid` and `probed` are printed
35+
//! as two lines because they are two answers — the config parsed, and the
36+
//! source was contacted — and only the second one is what "do these
37+
//! credentials work?" is asking. Secrets are never echoed back by the server,
38+
//! never printed here, and never shown while being typed.
3639
3740
use crate::client::ingest::{
3841
Capabilities, ConfigUpdate, ConnectorEntry, Datasource, DatasourceConfig, FamilyReference,
@@ -52,6 +55,12 @@ pub enum DatasourceCommands {
5255
/// Persists no datasource, config version, managed database, or secret —
5356
/// run it before `create` to see what the credentials can reach. The
5457
/// response carries family-specific discovery (schemas/tables, topics, …).
58+
///
59+
/// Two lines answer two questions. `valid` says the config and credentials
60+
/// were accepted; `probed` says whether the source was actually contacted,
61+
/// which is the one that means the credentials work. Some families have no
62+
/// read-only probe and can only be checked for shape — they report `valid:
63+
/// yes` with `probed: no` and a detail saying why.
5564
Validate {
5665
/// Source family — the shape of --config: sql, filesystem, iceberg,
5766
/// delta, ducklake, kafka, rest. Use `sql` for any SQL dialect (the
@@ -431,6 +440,7 @@ fn validate(workspace_id: &str, output: &str, family: &str, payload: ConfigArgs)
431440
} else {
432441
field("valid:", &"no".red().to_string());
433442
}
443+
field("probed:", &probed_cell(resp.probed));
434444
field("family:", &cell(resp.family.as_deref()));
435445
if let Some(d) = resp.detail.as_deref().filter(|d| !d.trim().is_empty()) {
436446
field("detail:", d);
@@ -449,6 +459,32 @@ fn validate(workspace_id: &str, output: &str, family: &str, payload: ConfigArgs)
449459
}
450460
}
451461

462+
/// PROBED — whether the source was contacted, printed on its own line beside
463+
/// `valid`.
464+
///
465+
/// A command called `validate` answering `valid: yes` is read as "the
466+
/// credentials work", and for some families it does not mean that: one with no
467+
/// read-only probe, and a credential shape no client can be built from, both
468+
/// check the config's SHAPE and connect to nothing. The response has always
469+
/// distinguished the two, but only through a detail line that appears when the
470+
/// probe was skipped — so the reader had to infer a negative from the absence
471+
/// of an explanation, and the reading that requires noticing nothing is the
472+
/// reading nobody performs. Stating it is what makes a preflight worth running.
473+
///
474+
/// Absent is its own answer rather than a `no`: a response that does not carry
475+
/// the field says nothing about the probe, and printing "no" there would be
476+
/// this command inventing the very claim it exists to report.
477+
fn probed_cell(probed: Option<bool>) -> String {
478+
use crossterm::style::{Color, Stylize};
479+
match probed {
480+
Some(true) => "yes — the source answered".with(Color::Green).to_string(),
481+
Some(false) => "no — nothing connected; the config's shape was checked"
482+
.with(Color::Yellow)
483+
.to_string(),
484+
None => "not reported".with(Color::DarkGrey).to_string(),
485+
}
486+
}
487+
452488
// --- create -----------------------------------------------------------------
453489

454490
fn create(
@@ -1429,6 +1465,51 @@ mod tests {
14291465
assert_eq!(config, serde_json::json!({"root_uri": "hdfs://nn/data"}));
14301466
}
14311467

1468+
/// `valid` and `probed` are two answers, and the second is the one a user
1469+
/// came for. A source that was never contacted must say so in words, not
1470+
/// by omitting a line somebody has to notice is missing.
1471+
#[test]
1472+
fn validate_states_whether_the_source_was_contacted() {
1473+
let connected = strip_ansi(&probed_cell(Some(true)));
1474+
assert!(connected.contains("yes"), "{connected}");
1475+
assert!(connected.contains("answered"), "{connected}");
1476+
1477+
// The case the line exists for: valid, and nothing reached.
1478+
let shape_only = strip_ansi(&probed_cell(Some(false)));
1479+
assert!(shape_only.starts_with("no"), "{shape_only}");
1480+
assert!(shape_only.contains("nothing connected"), "{shape_only}");
1481+
assert!(shape_only.contains("shape"), "{shape_only}");
1482+
1483+
// A response that does not carry the field is not a probe that failed.
1484+
let unknown = strip_ansi(&probed_cell(None));
1485+
assert!(unknown.contains("not reported"), "{unknown}");
1486+
assert!(!unknown.contains("no "), "{unknown}");
1487+
}
1488+
1489+
/// `-o json` is the struct serialized, so a field the struct drops is a
1490+
/// field a script cannot read however faithfully the service sends it.
1491+
#[test]
1492+
fn the_validate_response_carries_the_probe_through_to_json() {
1493+
let body = serde_json::json!({
1494+
"valid": true,
1495+
"family": "rest",
1496+
"normalized_config": {"base_url": "https://api.example.com"},
1497+
"discovered": null,
1498+
"probed": false,
1499+
"detail": "this family has no read-only probe",
1500+
});
1501+
let resp: crate::client::ingest::ValidateResponse = serde_json::from_value(body).unwrap();
1502+
assert!(resp.valid);
1503+
assert_eq!(resp.probed, Some(false));
1504+
1505+
let rendered = serde_json::to_value(&resp).unwrap();
1506+
assert_eq!(rendered["probed"], false);
1507+
assert_eq!(
1508+
strip_ansi(&probed_cell(resp.probed)),
1509+
"no — nothing connected; the config's shape was checked"
1510+
);
1511+
}
1512+
14321513
#[test]
14331514
fn discovered_lines_render_schema_qualified_tables() {
14341515
let d = serde_json::json!({

0 commit comments

Comments
 (0)