Skip to content

Commit 4e0b638

Browse files
committed
feat(ingest)!: destination flags follow the family
The service now takes `table` from the families that name the one table they write, and an optional `table_prefix` from the ones whose tables are named after the source. Sending the wrong one is refused with the field named, so the flags follow: `--dest-table` for the first, `--dest-table-prefix` for the second, each saying in help which families it applies to. Omitting the prefix keeps the raw source names, which is what someone syncing raw tables wants and what happens today. Listings present a prefix as a prefix, so nobody reads it as a table that will exist. Also: `ingest create --help` claimed a one-time ingest "runs immediately and reports its initial_run_id". It is always null -- the scheduler dispatches -- and the help now says what actually happens. And `datasource fields` dropped the field descriptions the service sends from its table view, which is where the useful text is: `root_uri`'s "object keys are recorded relative to it, so moving it starts a fresh read" was visible only under -o json.
1 parent b16ef26 commit 4e0b638

7 files changed

Lines changed: 606 additions & 128 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,13 @@ hotdata datasource list # ids, families, states
115115
# 3. Ingest — what to load, and where it lands. Once:
116116
hotdata ingest create --source "prod postgres" --table orders --database-id db_123
117117

118-
# …or on a schedule, kept fresh:
118+
# Source tables keep their own names. Put several under a common prefix —
119+
# orders lands as raw_orders, customers as raw_customers:
120+
hotdata ingest create --source "prod postgres" --database-id db_123 \
121+
--table orders --table customers --dest-table-prefix raw
122+
123+
# A source that lands ONE table is told that table's name instead, and can
124+
# be kept fresh on a schedule:
119125
hotdata ingest create --source ds_01J --type continuous --database-id db_123 \
120126
--format parquet --glob "orders/**/*.parquet" --dest-table orders_raw --every 5m
121127

‎src/client/ingest.rs‎

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,22 @@ impl IngestError {
6565
// or a follow-up hint should branch on — so print both.
6666
IngestError::Http { status, body } => {
6767
let message = util::api_error(body.clone());
68-
match util::error_code(body) {
68+
let mut rendered = match util::error_code(body) {
6969
Some(code) => format!("HTTP {status}: {message} ({code})"),
7070
None => format!("HTTP {status}: {message}"),
71+
};
72+
// The message says which payload was rejected; the details say
73+
// which field, and only together are they an instruction. One
74+
// line each, indented under it, because a rejected body can
75+
// name several fields and a joined sentence hides all but the
76+
// first.
77+
for (field, msg) in util::error_fields(body) {
78+
rendered.push_str(&match field.as_str() {
79+
"" => format!("\n {msg}"),
80+
f => format!("\n {f}: {msg}"),
81+
});
7182
}
83+
rendered
7284
}
7385
IngestError::Connection(e) => format!("connection error: {e}"),
7486
IngestError::Decode(e) => format!("malformed response: {e}"),
@@ -153,6 +165,20 @@ impl IngestError {
153165
.dark_grey()
154166
);
155167
}
168+
// The destination is per-family, and the pair of flags that
169+
// build it is the thing to re-read: a family lands one table
170+
// or a table per source table, never both, so a rejected
171+
// `table`/`table_prefix` is the other flag's job.
172+
Some("invalid_destination") => {
173+
eprintln!(
174+
"{}",
175+
"--dest-table names ONE table (buckets, Delta, a --raw-sql result); \
176+
--dest-table-prefix prefixes the tables a source that lands several \
177+
produces (SQL --table/--sql, Iceberg, DuckLake, Kafka, REST). The \
178+
family's own field reference: hotdata datasource fields <family>."
179+
.dark_grey()
180+
);
181+
}
156182
Some("immutable_ingest_definition") => {
157183
eprintln!(
158184
"{}",
@@ -1002,16 +1028,24 @@ pub struct FamiliesResponse {
10021028
/// `format: password` sits BESIDE the `anyOf`, not inside one of its members —
10031029
/// which is what makes an optional secret's marker easy to miss, and a secret
10041030
/// whose marker is missed is one the guided flow echoes to the terminal.
1031+
///
1032+
/// The `description` on a property is the service's own sentence about the
1033+
/// field, written on the model that validates it. Some properties have one and
1034+
/// some do not, which is the pair of cases the reference has to render — a
1035+
/// fixture where none did is how the field reference came to drop all of them
1036+
/// from its table and keep them only under `-o json`.
10051037
#[cfg(test)]
10061038
pub const FAMILY_REFERENCE_BODY: &str = r#"{
10071039
"family": "sql",
10081040
"config_schema": {
10091041
"additionalProperties": false,
10101042
"properties": {
10111043
"dialect": {"enum": ["postgres", "mysql", "duckdb"], "title": "Dialect",
1012-
"type": "string"},
1044+
"type": "string",
1045+
"description": "Which SQL engine this is. The rest of `config` is read in its terms."},
10131046
"host": {"anyOf": [{"type": "string"}, {"type": "null"}], "default": null,
1014-
"title": "Host"},
1047+
"title": "Host",
1048+
"description": "Server hostname. Omit for engines addressed by account or path."},
10151049
"port": {"anyOf": [{"maximum": 65535, "minimum": 1, "type": "integer"},
10161050
{"type": "null"}], "default": null, "title": "Port"},
10171051
"options": {"additionalProperties": true, "title": "Options", "type": "object"}
@@ -1040,7 +1074,8 @@ pub const FAMILY_REFERENCE_BODY: &str = r#"{
10401074
"schema": {"anyOf": [{"type": "string"}, {"type": "null"}],
10411075
"default": null, "title": "Schema"},
10421076
"tables": {"items": {"type": "string"}, "minItems": 1,
1043-
"title": "Tables", "type": "array"},
1077+
"title": "Tables", "type": "array",
1078+
"description": "Source tables to read. Each lands in a destination table of the same name."},
10441079
"limit": {"anyOf": [{"minimum": 1, "type": "integer"}, {"type": "null"}],
10451080
"default": null, "title": "Limit"}
10461081
},
@@ -1760,6 +1795,37 @@ mod tests {
17601795
}
17611796
}
17621797

1798+
/// A 422 about ONE field must say which field.
1799+
///
1800+
/// The message half names the payload and the family and stops there — by
1801+
/// design, since `details` is where the field paths are. Printing only the
1802+
/// message left a caller who sent a destination field the family does not
1803+
/// have with "invalid destination for family 'iceberg'" and nothing to act
1804+
/// on, which is the same amount of information as no error text at all.
1805+
#[test]
1806+
fn a_field_level_rejection_names_the_field_it_rejected() {
1807+
let err = IngestError::Http {
1808+
status: 422,
1809+
body: r#"{"error": {"code": "invalid_destination",
1810+
"message": "invalid destination for family 'iceberg'",
1811+
"details": {"errors": [
1812+
{"field": "table", "message": "Extra inputs are not permitted"}]}}}"#
1813+
.into(),
1814+
};
1815+
let rendered = err.message();
1816+
assert!(rendered.contains("invalid_destination"), "{rendered}");
1817+
assert!(
1818+
rendered.contains("\n table: Extra inputs are not permitted"),
1819+
"{rendered}"
1820+
);
1821+
// An envelope with no field errors is unchanged — one line, as before.
1822+
let plain = IngestError::Http {
1823+
status: 404,
1824+
body: r#"{"error":{"code":"run_not_found","message":"no run 'nope'"}}"#.into(),
1825+
};
1826+
assert_eq!(plain.message(), "HTTP 404: no run 'nope' (run_not_found)");
1827+
}
1828+
17631829
// --- catalog -----------------------------------------------------------
17641830

17651831
#[test]

‎src/commands/datasource.rs‎

Lines changed: 129 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,16 @@ fn print_family_reference(r: &FamilyReference) {
920920
print_schema(&r.credentials_schema);
921921
section("SELECTOR", "hotdata ingest create --selector");
922922
print_schema(&r.selector_schema);
923+
// The destination is per-family too — one family names a table, another
924+
// takes at most a prefix for the tables it lands. Read out of the
925+
// passthrough map rather than a field of our own: this section exists
926+
// BECAUSE the service is the only thing that can be right about which,
927+
// and a family whose reference does not describe it gets no section
928+
// rather than a CLI-side answer.
929+
if let Some(schema) = r.extra.get("destination_schema") {
930+
section("DESTINATION", "hotdata ingest create --destination");
931+
print_schema(schema);
932+
}
923933
println!();
924934
hint(&format!(
925935
"'hotdata datasource fields {} -o json' prints the JSON Schema itself, \
@@ -975,7 +985,10 @@ fn print_schema(schema: &serde_json::Value) {
975985
hint(" (no fields)");
976986
continue;
977987
}
978-
crate::output::table::print(&["FIELD", "TYPE", "REQUIRED", "DEFAULT"], &rows);
988+
crate::output::table::print(
989+
&["FIELD", "TYPE", "REQUIRED", "DEFAULT", "DESCRIPTION"],
990+
&rows,
991+
);
979992
}
980993
}
981994

@@ -1034,11 +1047,33 @@ fn field_rows(schema: &serde_json::Value) -> Vec<Vec<String>> {
10341047
type_label(prop),
10351048
yes_no(required.iter().any(|r| r == name)),
10361049
default_cell(prop),
1050+
description_cell(prop),
10371051
]
10381052
})
10391053
.collect()
10401054
}
10411055

1056+
/// What the field is FOR, as the service describes it.
1057+
///
1058+
/// Whole, not summarised: these sentences are written on the models that
1059+
/// validate the request and they carry the part no type or default can — which
1060+
/// engines want a host, what a root uri does to object keys, what a prefix is
1061+
/// relative to. Trimming them to a column width would drop the second sentence,
1062+
/// which is where that usually lives. The table wraps this cell instead, and a
1063+
/// terminal too narrow to hold it says so and points at `-o json`.
1064+
///
1065+
/// Newlines are collapsed because a description written across source lines is
1066+
/// one sentence, and a cell that kept the breaks would set its own row height
1067+
/// rather than wrap to the width it was given.
1068+
fn description_cell(prop: &serde_json::Value) -> String {
1069+
match prop.get("description").and_then(|d| d.as_str()) {
1070+
Some(text) if !text.trim().is_empty() => {
1071+
text.split_whitespace().collect::<Vec<_>>().join(" ")
1072+
}
1073+
_ => "-".into(),
1074+
}
1075+
}
1076+
10421077
fn required_names(schema: &serde_json::Value) -> Vec<String> {
10431078
schema
10441079
.get("required")
@@ -1461,21 +1496,60 @@ mod tests {
14611496
}
14621497

14631498
#[test]
1464-
fn field_rows_carry_the_type_the_requirement_and_the_default() {
1499+
fn field_rows_carry_the_type_the_requirement_the_default_and_the_description() {
14651500
let reference = sql_reference();
14661501
let rows = field_rows(&reference.config_schema);
14671502

14681503
// An enum's members ARE the type: "string" would leave the caller
1469-
// guessing at the one thing a 422 will be about.
1504+
// guessing at the one thing a 422 will be about. The description is
1505+
// the service's own sentence about the field, and it is the half of
1506+
// the row that says what the field is FOR — it used to reach `-o json`
1507+
// and nothing else.
14701508
assert_eq!(
14711509
row(&rows, "dialect"),
1472-
["dialect", "postgres | mysql | duckdb", "yes", "-"]
1510+
[
1511+
"dialect",
1512+
"postgres | mysql | duckdb",
1513+
"yes",
1514+
"-",
1515+
"Which SQL engine this is. The rest of `config` is read in its terms."
1516+
]
14731517
);
14741518
// Optional fields arrive as a union against null. The null member is
14751519
// the REQUIRED column's job and must not reach the type.
1476-
assert_eq!(row(&rows, "host"), ["host", "string", "no", "-"]);
1477-
assert_eq!(row(&rows, "port"), ["port", "integer", "no", "-"]);
1478-
assert_eq!(row(&rows, "options"), ["options", "object", "no", "-"]);
1520+
assert_eq!(
1521+
row(&rows, "host"),
1522+
[
1523+
"host",
1524+
"string",
1525+
"no",
1526+
"-",
1527+
"Server hostname. Omit for engines addressed by account or path."
1528+
]
1529+
);
1530+
// A field the service describes with nothing gets a dash, like every
1531+
// other empty cell in the CLI — never a blank column.
1532+
assert_eq!(row(&rows, "port"), ["port", "integer", "no", "-", "-"]);
1533+
assert_eq!(row(&rows, "options"), ["options", "object", "no", "-", "-"]);
1534+
}
1535+
1536+
#[test]
1537+
fn a_description_written_across_lines_is_one_cell() {
1538+
// Pydantic descriptions are written as adjacent source strings and
1539+
// arrive with the breaks in them. A cell keeping those breaks sets its
1540+
// own row height instead of wrapping to the width the table gave it.
1541+
let prop = serde_json::json!({
1542+
"type": "string",
1543+
"description": "Full DSN, e.g. postgresql://user@host/db.\nReplaces the\n fields above."
1544+
});
1545+
assert_eq!(
1546+
description_cell(&prop),
1547+
"Full DSN, e.g. postgresql://user@host/db. Replaces the fields above."
1548+
);
1549+
assert_eq!(
1550+
description_cell(&serde_json::json!({"type": "string"})),
1551+
"-"
1552+
);
14791553
}
14801554

14811555
#[test]
@@ -1489,17 +1563,60 @@ mod tests {
14891563
let (tables_label, tables) = &variants[0];
14901564
assert_eq!(tables_label.as_deref(), Some("mode = tables"));
14911565
let rows = field_rows(tables);
1492-
assert_eq!(row(&rows, "tables"), ["tables", "string[]", "yes", "-"]);
1493-
assert_eq!(row(&rows, "mode"), ["mode", "tables", "no", "tables"]);
1494-
assert_eq!(row(&rows, "schema"), ["schema", "string", "no", "-"]);
1566+
assert_eq!(
1567+
row(&rows, "tables"),
1568+
[
1569+
"tables",
1570+
"string[]",
1571+
"yes",
1572+
"-",
1573+
"Source tables to read. Each lands in a destination table of the same name."
1574+
]
1575+
);
1576+
assert_eq!(row(&rows, "mode"), ["mode", "tables", "no", "tables", "-"]);
1577+
assert_eq!(row(&rows, "schema"), ["schema", "string", "no", "-", "-"]);
14951578

14961579
// Each form has its own required set: `sql` is required in the query
14971580
// form and absent from the other, which merging them would hide.
14981581
let (query_label, query) = &variants[1];
14991582
assert_eq!(query_label.as_deref(), Some("mode = query"));
15001583
let rows = field_rows(query);
1501-
assert_eq!(row(&rows, "sql"), ["sql", "string", "yes", "-"]);
1502-
assert_eq!(row(&rows, "mode"), ["mode", "query", "yes", "-"]);
1584+
assert_eq!(row(&rows, "sql"), ["sql", "string", "yes", "-", "-"]);
1585+
assert_eq!(row(&rows, "mode"), ["mode", "query", "yes", "-", "-"]);
1586+
}
1587+
1588+
/// The destination is per-family, and a family that describes one is
1589+
/// carried through the passthrough map rather than a field of the CLI's:
1590+
/// which of `table` / `table_prefix` a family takes is the service's
1591+
/// answer, and a CLI-side copy of it is the thing this command replaces.
1592+
#[test]
1593+
fn a_family_that_describes_its_destination_is_carried_through() {
1594+
let reference = sql_reference();
1595+
assert!(
1596+
reference.extra.get("destination_schema").is_none(),
1597+
"the pinned body describes no destination; the section must not invent one"
1598+
);
1599+
1600+
let described: FamilyReference = serde_json::from_str(
1601+
r#"{"family": "iceberg", "destination_schema": {
1602+
"properties": {"table_prefix": {"type": "string"}},
1603+
"additionalProperties": false, "type": "object"}}"#,
1604+
)
1605+
.unwrap();
1606+
let schema = described
1607+
.extra
1608+
.get("destination_schema")
1609+
.expect("carried through untyped");
1610+
assert_eq!(
1611+
field_rows(schema),
1612+
vec![vec![
1613+
"table_prefix".to_string(),
1614+
"string".into(),
1615+
"no".into(),
1616+
"-".into(),
1617+
"-".into()
1618+
]]
1619+
);
15031620
}
15041621

15051622
#[test]

0 commit comments

Comments
 (0)