Skip to content

fix(cli): clean errors for malformed inputs and honest dbt-gate exit codes - #351

Open
kevincostner17 wants to merge 1 commit into
mainfrom
fix/cli-malformed-inputs-and-exit-codes
Open

kevincostner17 wants to merge 1 commit into
mainfrom
fix/cli-malformed-inputs-and-exit-codes

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Summary

  • CLIs print tracebacks for malformed config/suite/manifest files; validate exits 1, not 2 #289: malformed input files printed tracebacks.
    • freshdata clean --config now reports these as a one-line freshdata: error: ... naming the file, with exit 1 (the existing convention for bad input files):
      • invalid YAML or JSON
      • a top level that isn't an object
      • a clean/enterprise section that isn't an object
      • unknown keys (a typo in a masking rule or clean option)
    • freshdata validate --suite/--contract with a non-object file now exits 2 ("could not load rules") instead of 1 ("validation failed"). ValidationSuite.from_dict rejects non-mappings with ValueError.
    • dbt-gate now catches (OSError, ValueError). Invalid JSON, a directory path, or a file that isn't a manifest now gives a one-line dbt-gate: error: ... with exit 1, like a missing manifest.
  • freshdata clean exits 1 after writing output when stdout isn't UTF-8 (summary prints '→') #295: exit 1 when stdout isn't UTF-8. New _safe_print retries with errors="replace" on UnicodeEncodeError. It's used for the clean summary and notes and the validate verdict, so a cp1252/ascii stdout no longer turns a passed gate (outputs already written) into exit 1.
  • dbt-gate --fail exits 0 when the manifest has no models or is the wrong file #296: dbt-gate --fail passed when nothing was gated.
    • gate_manifest raises ValueError when the file has no nodes mapping (e.g. run_results.json).
    • all_passed is false when models_processed == 0.
    • dbt-gate prints a "no models were gated" note to stderr, and --fail exits 1.
  • dbt-gate counts ephemeral models as failures, so --fail always fails when they exist #249: ephemeral models were counted as failures. Models with config.materialized == "ephemeral" or config.enabled == false are skipped. They're listed under a new "skipped" summary key and not counted in models_processed or failed_models.

Behaviour changes:

  • The dbt-gate summary has a new skipped key.
  • all_passed is false when nothing was gated.
  • validate exits 2 for non-object rule files.

Tests

  • New tests/test_cli_malformed_inputs.py:
    • invalid, non-object and typo'd YAML/JSON configs
    • non-object sections and masking entries
    • unknown clean options
    • validate exits 2 for non-object --suite and --contract files
    • ValidationSuite.from_dict rejects non-mappings
    • clean and validate on an ascii stdout: a passed gate exits 0, a failed gate still exits 1
  • tests/test_integrations/test_dbt.py:
    • malformed manifests and a directory path give one-line errors
    • a file that isn't a manifest raises ValueError
    • a zero-model manifest doesn't pass, and --fail exits 1
    • ephemeral and disabled models are skipped, with --fail exiting 0 when the real tables pass

Verification

  • ruff check .: all checks passed
  • mypy src/freshdata: no issues found in 202 source files
  • pytest (-m "not online and not large"), py3.12 / pandas 2.3.3: 4272 passed, 6 skipped
  • pytest, py3.9 / pandas 1.5.3: 4268 passed, 10 skipped
  • All four issue reproductions checked before and after the fix.

Closes #289
Closes #295
Closes #296
Closes #249

…codes

Malformed config/suite/manifest files (#289):
- freshdata clean --config: invalid YAML/JSON, a non-object top level, a
  non-object "clean"/"enterprise" section, and unknown keys (TypeError from
  MaskingRule(**rule) or merge_options) now raise ValueError naming the file.
  main() reports them as one-line "freshdata: error: ..." with exit 1, the
  existing convention for bad input files. PyYAML's multi-line message is
  collapsed to one line.
- freshdata validate: ValidationSuite.from_dict rejects non-mappings with
  ValueError, and --contract checks the top level too, so non-object rule
  files exit 2 ("could not load rules") instead of 1 ("validation failed").
- dbt-gate: catch (OSError, ValueError), so invalid JSON, a directory path
  and non-manifest files are one-line errors with exit 1, like a missing
  manifest.

Non-UTF-8 stdout (#295):
- Add _safe_print(), which reprints with errors="replace" on
  UnicodeEncodeError. It is used for the clean summary/notes and the
  validate verdict, so a cp1252/ascii stdout no longer turns a passed gate
  (with outputs already written) into exit 1. EnterpriseResult.summary is
  unchanged.

Nothing gated must not pass (#296):
- gate_manifest raises ValueError when the file has no "nodes" mapping.
- all_passed is now false when models_processed == 0; dbt-gate prints a
  "no models were gated" note to stderr, and --fail exits 1.

Ephemeral models (#249):
- gate_manifest skips models with config.materialized == "ephemeral" or
  config.enabled == false and lists them under a new "skipped" key. They
  are not counted in models_processed or failed_models.

Closes #289
Closes #295
Closes #296
Closes #249
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 802cfc84-5529-4605-bbeb-43782b28dd30


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment