feat(cube-cli): reintroduce cube validate as a Cloud data-model check - #11595
feat(cube-cli): reintroduce cube validate as a Cloud data-model check#11595paveltiunov wants to merge 3 commits into
cube validate as a Cloud data-model check#11595Conversation
The old `cubejs validate` compiled the model locally, which meant it could
only ever check what a local checkout plus a local install could express —
not the environment variables, drivers or dependencies the model actually
runs against.
`cube validate <deployment>` instead asks the branch's own Cube runtime for
`GET /cubejs-api/v1/meta`, the same call the console makes in dev mode, via
the new `GET /build/api/v1/deployments/{id}/data-model/validate` endpoint. So
the verdict is by construction the one that branch's API would give, and
naming the branch is what picks the runtime:
cube validate 42 # the deploy branch (production)
cube validate 42 --branch my-branch # a specific branch
cube validate 42 --dev-mode # the active dev-mode working copy
It exits non-zero with the compiler's errors, per file, so it works as a CI
gate; `--json` gives the same report machine-readably.
Ref CUB-3782
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Claude finished @paveltiunov's task in 2m 59s —— View job Review:
|
| # | Severity | Where | Issue |
|---|---|---|---|
| 1 | Medium | validate.rs:30-38 |
Non-object errors[] entries (plain strings) render as blank lines — a failed validation with zero actionable output |
| 2 | Low | validate.rs:58 |
Missing branchName in the response yields Data model on is valid |
| 3 | Low | validate.rs:74-91 |
valid: false with empty errors prints failed to compile: followed by nothing, then has 0 compilation error(s) |
All three are the same root theme: the response contract lives in a different repo (cubedevinc/cubejs-enterprise#14215) and this command reads it optimistically. None are blocking — worst case is a confusing message, never a false pass.
Other observations (no inline comment)
- Telemetry noise. The
bail!on an invalid model flows throughmain.rs:293-297, which emits anErrortelemetry event andsuccess: false. A CI gate firing as designed will therefore look identical to a CLI malfunction in the metrics. If you care about that distinction, an exit-code-only path (or an error property marking it expected) would separate "the CLI broke" from "the model didn't compile." rust/cube-cli/README.md:129-154has a command table introduced by "Every endpoint of the Console Server public API is covered" with novalidaterow.deployis missing there too, so this is pre-existing drift rather than something this PR introduced — but adding a row is a one-liner if you're touching it.- Test coverage is limited to
format_error, which is reasonable given the rest ofcommand()is I/O-bound and the crate has no HTTP test harness. If findings 1 and 3 are addressed, the string-error case and the empty-errorscase are both testable as pure functions and worth a line each. - Not verified locally:
cargo test/cargo fmt --check/cargo clippyneed approval I don't have in this environment, so I'm taking the PR description's "20 tests pass" at face value. The suggested diffs above are written to compile as drop-ins but haven't been run through rustc here.
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.
|
Claude finished @paveltiunov's task in 1m 2s —— View job Re-review of
|
| Prior finding | Status |
|---|---|
Non-object errors[] entries render as blank lines |
Fixed — format_error (validate.rs:37-51) now handles a bare string, a message-only object, a file-only object, and falls back to the entry's own JSON. The 4-arm match (file.is_empty(), message.is_empty()) covers strictly more shapes than the format!("{file}{message}") I suggested, which would still have produced an empty line for {"code": 7}. |
Missing branchName yields on is valid |
Fixed — extracted into branch_label (validate.rs:60-71) with --dev-mode getting its own label, as suggested. |
!valid with empty errors prints a header promising a list, then nothing |
Fixed — separate else if errors.is_empty() branch (:109-116) and a distinct bail! (:130-135). |
I traced the semantics rather than trusting the tests: output::field returns "" both when the key is missing and when its value is null (output.rs:38, 50), so {"fileName": null, "message": "no sql"} correctly takes the (true, false) arm, and Value::Null takes (true, true) → "null". The two new test fns (validate.rs:165-179, :189-208) assert exactly those boundary shapes, and the args() helper keeps branch_label testable without an HTTP harness. Coverage is now good for everything in this file that isn't I/O.
Remaining observations (nothing inline, nothing blocking)
- Cosmetic double-report in the empty-errors path. In human mode a
valid: false/ no-errors response prints✗ Data model on X could not be validated.and thenerror: data model on X could not be validated (the API reported a failure without any compilation errors). The stderr line adds nothing thebail!doesn't already say, unlike the error-list case where it heads a list. Dropping it would leave the parenthetical as the single, more informative line — purely a polish call, and the current output isn't wrong. - Telemetry noise (carried over from the last pass, unchanged): the
bail!on an invalid model flows throughmain.rs:293-297, so a CI gate firing as designed emits anErrorevent indistinguishable from a CLI malfunction. Worth a follow-up if those metrics matter; not something to hold this PR for. errorsnon-empty withvalid: trueis ignored (the success branch wins). Correct given the server owns the verdict, just noting it's a deliberate asymmetry.- README/docs — the
validaterow I flagged as missing is now inrust/cube-cli/README.md:133, andcli.mdxhas both the section and the command-reference row.deployis still absent from the README table, but that's pre-existing drift.
Not verified locally
cargo test / cargo fmt --check / cargo clippy require approval this environment doesn't grant, so I reviewed by reading. I'm taking the PR description's "20 tests pass" at face value; my correctness claims above come from tracing format_error and branch_label against output.rs, not from a run.
Housekeeping
Resolved all three of my prior review threads — the code each pointed at has been rewritten and the concerns no longer apply.
The empty-errors path printed a stderr line and then a bail! saying the same thing, unlike the error-list case where the stderr line heads a list. Keeping only the bail! leaves one line, and it's the more informative one — it says the API reported a failure without compilation errors, which is what points the operator at the runtime rather than the model.
|
Claude finished @paveltiunov's task in 45s —— View job Re-review of
|
valid |
errors |
stdout | stderr |
|---|---|---|---|
| true | — | ✓ Data model on X is valid (N cubes) |
— |
| false | non-empty | — | header + one line per error, then error: … has N compilation error(s) |
| false | empty | — | error: data model on X could not be validated (the API reported a failure without any compilation errors) — one line |
| any | any, --json |
full report | the bail! line only |
No arm can now emit a header with nothing under it, and no arm emits two lines saying the same thing. Exit code is unchanged in all failure states — main.rs:304-307 still turns the bail! into exit(1), including under --json, which is what makes the CI gate identical in both modes.
Nothing else in the file moved: format_error (:37-51), branch_label (:60-71) and the 4 tests (:146-204) are byte-identical to a721ee6, which I already reviewed and traced against output::field's missing-vs-null semantics. OwoColorize is still used (:113), so removing the other .red() call didn't orphan the import.
Carried-over observations (unchanged, none blocking)
- Telemetry noise. Still the one thing I'd consider a follow-up: the
bail!on an invalid model flows throughmain.rs:293-297, which emits anErrorevent withsuccess: false. A CI gate firing exactly as designed is indistinguishable in metrics from the CLI malfunctioning. Only worth doing if those metrics are actually read. errorsnon-empty withvalid: trueis ignored — deliberate, the server owns the verdict.deployis missing from therust/cube-cli/README.mdcommand table — pre-existing drift, not this PR's.validateis present at:133.
Housekeeping
All three of my earlier threads were already resolved in the previous round; no threads to resolve and no new inline comments to post — this commit introduced nothing to comment on.
Not verified locally
cargo test / cargo fmt --check / cargo clippy need approval this environment doesn't grant (the call was blocked), so this is a read-based review. The claims above come from tracing the control flow and output.rs, not from a run; I'm taking the PR description's passing-test claim at face value. CI on the branch is the authority there.
Check List
Issue Reference this PR resolves
Linear: CUB-3782
Server side: cubedevinc/cubejs-enterprise#14215 — the endpoint this command calls. Merge that first; until it ships this command 404s.
Description of Changes Made
Brings
validateback to the CLI, as a Cloud check rather than the local compile the oldcubejs validatedid.The compile happens where the model runs: the command calls
GET /build/api/v1/deployments/{id}/data-model/validate, which asks the branch's own Cube runtime for/meta— the same call the console makes in dev mode. That checks the model against the deployment's real environment variables, drivers and dependencies, which a local compile cannot, and it's what makes--branch/--dev-modemeaningful: each is served by its own runtime.--dev-modevalidates your uncommitted working copy, before you commit it.It exits non-zero with the compiler's errors, one per line and prefixed with the file the compiler blamed, so it works as a CI gate:
Errors go to stderr and
--jsonprints the full report on stdout (valid,errors[],cubesCount), so the exit code gates the same way in both modes.The legacy
cubejs validateinpackages/cubejs-cliis untouched — this is the newcubebinary only.docs-mintlify/reference/cli.mdxgains a "Validate the data model" section and a command-reference row.cargo fmt --check,cargo clippy --all-targets -- -D warningsandcargo testall pass (20 tests, including new coverage for the per-file error rendering).Generated by Claude Code