Skip to content

Commit a721ee6

Browse files
committed
fix(cube-cli): never print a validation failure with nothing to act on
Review of the paired PR. All three are the same shape: the response contract lives in cubedevinc/cubejs-enterprise and this command ships separately, so an entry that doesn't match what it expects still has to print as something. A blank line is the one output `validate` must never produce — printing the errors IS the command. - `format_error` renders a bare-string entry as itself and a half-filled object as whichever half it has, falling back to the entry's own JSON rather than the empty string `output::field` would give for both lookups. - A response without `branchName` no longer yields "Data model on is valid": it falls back to the branch the caller asked for, or a generic label for `--dev-mode`, where the personal `dev-…` name only exists server-side. - `valid: false` with no errors said "failed to compile:" and then listed nothing, followed by "has 0 compilation error(s)". It now says the model could not be validated, which points at the runtime — where the answer is. Still fails closed: a report this command can't read is not evidence the model compiles. Adds a `validate` row to the README's command table, and unit tests for the new pure functions.
1 parent 2a8ca36 commit a721ee6

2 files changed

Lines changed: 100 additions & 5 deletions

File tree

rust/cube-cli/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ Every endpoint of the Console Server public API is covered:
130130
|---|---|
131131
| `deployments` | list, get, create (`--bootstrap` scaffolds + builds a serving deployment), update (`--release-channel`, `--release-channel-version`), settings, versions, delete, token, advance-step, reset-step |
132132
| `regions` | list available deployment regions |
133+
| `validate` | compile a deployment's data model and report the compiler's errors; exits non-zero so it gates CI. `--branch` picks a branch, `--dev-mode` your active dev-mode working copy; neither validates the deploy branch |
133134
| `logs` | tail deployment pod logs (`--pod`, `-c/--container`; defaults to the Cube API container) |
134135
| `github` (`gh`) | status, installations, repos, branches, connect (import a repo into a deployment + first build) |
135136
| `data-model` (`dm`) | list, get, put, delete, rename files; branches, create-branch, enable-branch, disable-branch, dev-mode, exit-dev-mode, commit, pull. File writes only land on a **dev-mode branch**: `dev-mode <branch>` forks a personal `dev-…` branch and prints its name — pass that via `--branch` (or omit `--branch` to use your active dev-mode branch); puts to any other branch are rejected by the API. `enable-branch` / `disable-branch` toggle whether a shared branch's staging environment stays always active (vs. only while viewed in the UI); `branches` reports it as `ENABLED` and `environments list --type staging` lists the enabled ones |

rust/cube-cli/src/commands/validate.rs

Lines changed: 99 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,46 @@ pub struct Args {
2727

2828
/// Render one compilation error as `<file>: <message>`, or just the message
2929
/// when the compiler didn't attribute it to a file.
30+
///
31+
/// The endpoint reports each error as `{ fileName?, message }`, but the CLI and
32+
/// the server ship from separate repos on separate cadences, so an entry that
33+
/// doesn't match that shape still has to print as something: a blank line is
34+
/// the one output this command must never produce — printing the errors IS the
35+
/// command. So a bare string renders as itself, a half-filled object renders as
36+
/// whichever half it has, and anything else falls back to its own JSON.
3037
fn format_error(error: &Value) -> String {
38+
if let Value::String(message) = error {
39+
return message.clone();
40+
}
41+
3142
let message = output::field(error, "message");
3243
let file = output::field(error, "fileName");
33-
if file.is_empty() {
34-
message
35-
} else {
36-
format!("{file}: {message}")
44+
45+
match (file.is_empty(), message.is_empty()) {
46+
(false, false) => format!("{file}: {message}"),
47+
(true, false) => message,
48+
(false, true) => file,
49+
(true, true) => error.to_string(),
50+
}
51+
}
52+
53+
/// What to call the validated branch in user-facing output.
54+
///
55+
/// The server echoes the branch it resolved, which is the only source for the
56+
/// `--dev-mode` case (the personal `dev-…` name is server-side). A
57+
/// differently-versioned one that doesn't echo it must not turn every message
58+
/// into "on is valid" — fall back to what the caller asked for, and to a
59+
/// generic label when the caller named nothing either.
60+
fn branch_label(res: &Value, args: &Args) -> String {
61+
let echoed = output::field(res, "branchName");
62+
if !echoed.is_empty() {
63+
return echoed;
64+
}
65+
66+
match (&args.branch, args.dev_mode) {
67+
(Some(branch), _) => branch.clone(),
68+
(None, true) => "the dev-mode branch".to_string(),
69+
(None, false) => "the deploy branch".to_string(),
3770
}
3871
}
3972

@@ -55,7 +88,9 @@ pub async fn command(args: Args, ctx: &Ctx) -> Result<()> {
5588
)
5689
.await?;
5790

58-
let branch = output::field(&res, "branchName");
91+
let branch = branch_label(&res, &args);
92+
// Absent `valid` fails closed: a report this command can't read is not
93+
// evidence the model compiles, and the whole point is gating CI on it.
5994
let valid = res.get("valid").and_then(Value::as_bool).unwrap_or(false);
6095
let errors = res
6196
.get("errors")
@@ -71,6 +106,14 @@ pub async fn command(args: Args, ctx: &Ctx) -> Result<()> {
71106
Some(n) => output::success(&format!("Data model on {branch} is valid ({n} cubes)")),
72107
None => output::success(&format!("Data model on {branch} is valid")),
73108
}
109+
} else if errors.is_empty() {
110+
// A failure the API couldn't itemize. Saying "failed to compile:" here
111+
// would promise a list and then print nothing; point at the runtime
112+
// instead, which is where the answer actually is.
113+
eprintln!(
114+
"{} Data model on {branch} could not be validated.",
115+
"✗".red()
116+
);
74117
} else {
75118
// Compilation errors go to stderr so `cube validate --json` stays
76119
// machine-readable on stdout and a human run stays readable when
@@ -84,6 +127,12 @@ pub async fn command(args: Args, ctx: &Ctx) -> Result<()> {
84127
if !valid {
85128
// Non-zero exit is the point of the command in CI, so it holds in
86129
// --json mode too, where the report above was printed as JSON.
130+
if errors.is_empty() {
131+
bail!(
132+
"data model on {branch} could not be validated \
133+
(the API reported a failure without any compilation errors)"
134+
);
135+
}
87136
bail!(
88137
"data model on {branch} has {} compilation error(s)",
89138
errors.len()
@@ -112,4 +161,49 @@ mod tests {
112161
"no sql"
113162
);
114163
}
164+
165+
#[test]
166+
fn an_entry_that_is_not_the_expected_object_still_prints_something() {
167+
// A blank line is the one output this command must never produce, so
168+
// every shape a differently-versioned server could send has to render.
169+
assert_eq!(
170+
format_error(&json!("Orders cube: no sql")),
171+
"Orders cube: no sql"
172+
);
173+
assert_eq!(
174+
format_error(&json!({"fileName": "model/cubes/orders.yml"})),
175+
"model/cubes/orders.yml"
176+
);
177+
assert_eq!(format_error(&json!({"code": 7})), r#"{"code":7}"#);
178+
assert_eq!(format_error(&Value::Null), "null");
179+
}
180+
181+
fn args(branch: Option<&str>, dev_mode: bool) -> Args {
182+
Args {
183+
deployment: 1,
184+
branch: branch.map(str::to_string),
185+
dev_mode,
186+
}
187+
}
188+
189+
#[test]
190+
fn the_branch_the_server_echoes_wins() {
191+
// The `--dev-mode` name only exists server-side, so the echo is the
192+
// only way to report which branch was actually validated.
193+
let res = json!({"branchName": "dev-pavel-feature", "valid": true});
194+
assert_eq!(branch_label(&res, &args(None, true)), "dev-pavel-feature");
195+
assert_eq!(
196+
branch_label(&res, &args(Some("feature"), false)),
197+
"dev-pavel-feature"
198+
);
199+
}
200+
201+
#[test]
202+
fn a_response_without_a_branch_never_leaves_a_hole_in_the_message() {
203+
// "Data model on is valid" is the failure this guards against.
204+
let res = json!({"valid": true});
205+
assert_eq!(branch_label(&res, &args(Some("feature"), false)), "feature");
206+
assert_eq!(branch_label(&res, &args(None, true)), "the dev-mode branch");
207+
assert_eq!(branch_label(&res, &args(None, false)), "the deploy branch");
208+
}
115209
}

0 commit comments

Comments
 (0)