From 233f2636a2de29588b0eb5d4dbc7c1103a4eb9c3 Mon Sep 17 00:00:00 2001 From: rlougee Date: Tue, 21 Jul 2026 15:48:52 -0400 Subject: [PATCH 1/3] feat(ol_dbt_cli): add --old-glue/--new-glue/--refresh-glue to diff, and local snapshot command - ol-dbt diff --old-glue/--new-glue compares a local build against the real, already-materialized production table via its Glue-registered view, without requiring a rebuild of that side. --new now defaults to --old's value for this common case. --refresh-glue re-registers Glue views first, since production rebuilds regularly invalidate previously registered ones (stale S3 metadata 404s). - Fixes discovered along the way: a silently-swallowed dbt error in raw column resolution, a double-LIMIT DuckDB parser error, a long-standing f-string brace-escaping bug that broke every per-column comparison with a primary key, and an IcebergScan/UNION-ALL limitation worked around by materializing the glue-registered side into a real table before comparing. - Narrows per-column comparison queries to primary key + compared column instead of selecting every column, and reformats sample mismatches to show only the differing fields per key instead of full-row JSON dumps. - New `ol-dbt local snapshot` command copies a model's current build into a plain table under a new name, for before/after comparisons of the same model across a code change without involving Glue/production. Co-Authored-By: Claude Sonnet 5 --- src/ol_dbt_cli/ol_dbt_cli/commands/diff.py | 602 ++++++++++++++++-- .../ol_dbt_cli/commands/local_dev.py | 69 ++ src/ol_dbt_cli/tests/test_diff.py | 434 ++++++++++++- src/ol_dbt_cli/tests/test_local_dev.py | 30 + 4 files changed, 1088 insertions(+), 47 deletions(-) diff --git a/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py b/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py index 7b4dbc4d4..3b52c8973 100644 --- a/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py +++ b/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py @@ -16,6 +16,16 @@ ``--exclude-columns``. * The comparison itself is run via ``dbt show --inline`` invoking audit_helper macros with ``--output json``; results are parsed tolerantly from stdout. +* ``--old``/``--new`` normally resolve through ``ref()``, which requires a real + dbt model in the manifest. ``--old-raw``/``--new-raw`` bypass that, building a + literal ``api.Relation.create(...)`` instead — for relations dbt doesn't know + about: an already-materialized table sitting in a different schema/database + than the current --target. +* ``--old-glue``/``--new-glue`` are a variant of ``--old-raw``/``--new-raw`` + specifically for the ``glue__{glue_database}__{table}`` DuckDB views created by + ``ol-dbt local register`` — live views onto the real production Iceberg table, + not a copy. Pass just the Glue database name; ``--old``/``--new`` stay the bare + table name on both sides instead of the full ``glue__...`` identifier. Driveable both by hand and by CI (Phase 2 auto-diff vs a base ref). See docs/specs/DBT_WAREHOUSE_CI_QA_SPEC.md §3. @@ -113,6 +123,14 @@ class DiffResult: column_mismatches: list[ColumnMismatch] sample_mismatches: list[dict[str, Any]] verdict: Verdict + # Text-output-only labels annotating each side's source (e.g. "name (glue: + # db)") when --old/new-raw or --old/new-glue make `old`/`new` ambiguous — + # identical to `old`/`new` for a plain ref()-vs-ref() diff. Kept separate + # from `old`/`new` so JSON output stays a stable, bare identifier. Uses + # parens, not brackets — rich.console.print() parses `[...]` as markup and + # silently swallows anything that isn't a recognized style tag. + old_label: str + new_label: str notes: list[str] = field(default_factory=list) @@ -146,6 +164,65 @@ def _resolve_columns( return set() +def _resolve_raw_columns( + name: str, + dbt_dir: Path, + target: str, + *, + database: str | None = None, + schema: str | None = None, + glue_database: str | None = None, +) -> tuple[set[str], str | None]: + """Resolve a raw (non-``ref()``) relation's columns by sampling one row. + + Raw relations (Glue-registered views, an already-materialized table in + another schema) have no manifest/YAML metadata to resolve columns from + statically, so this samples a live row and reads its keys instead. + + Returns ``(columns, error)``. *error* carries the dbt failure message when + the sampling query itself fails (e.g. the relation doesn't exist) — the + caller must surface it rather than let an empty column set masquerade as + "unparsed / not in manifest", which is misleading for a raw relation. + """ + expr = _relation_jinja(name, raw=True, database=database, schema=schema, glue_database=glue_database) + # Rendered into a Jinja template compiled by dbt (not executed as a raw query + # here), so the S608 heuristic is a false positive — same as _compare_column_sql. + # No literal LIMIT here: `dbt show` already wraps the compiled query in its own + # limiting logic via --limit below; a second, literal `limit 1` in the SQL text + # collides with that wrapper and breaks the DuckDB parser. + inline_sql = f"select * from {{{{ {expr} }}}}" # noqa: S608 + try: + rows = _run_dbt_show(inline_sql, dbt_dir, target, limit=1) + except RuntimeError as exc: + return set(), str(exc) + return ({c.lower() for c in rows[0]} if rows else set()), None + + +def _materialize_glue_relation(name: str, glue_database: str, dbt_dir: Path, target: str) -> str: + """Copy a Glue-registered ``iceberg_scan()`` view into a real local DuckDB table. + + DuckDB's Iceberg extension can run a plain ``SELECT``/``COUNT(*)`` against a + live ``glue__...`` view (that's why column resolution works) but not the + ``UNION ALL``/``EXCEPT`` set operations audit_helper's compare macros + generate — that fails with ``Not implemented Error: IcebergScan + serialization not implemented``. Copying the view into a real table first + sidesteps the limitation entirely. + + Uses ``create or replace table`` with a name deterministic in + (*glue_database*, *name*), so repeated diffs reuse/refresh the same scratch + table instead of accumulating new ones. Returns the scratch table's bare + identifier (materialized in the target's own database/schema). + """ + scratch_name = f"_ol_dbt_diff_scratch__{glue_database}__{name}" + src_expr = _relation_jinja(name, glue_database=glue_database) + dst_expr = _relation_jinja(scratch_name, raw=True) + ctas_sql = f"create or replace table {{{{ {dst_expr} }}}} as select * from {{{{ {src_expr} }}}}" # noqa: S608 + # limit=-1: dbt show's own --limit wrapping would otherwise truncate the + # CTAS to N rows -- the whole point here is a full, unwrapped copy. + _run_dbt_show(ctas_sql, dbt_dir, target, limit=-1) + return scratch_name + + def reconcile_columns( old_cols: set[str], new_cols: set[str], @@ -276,6 +353,38 @@ def _jinja_list(values: list[str]) -> str: return f"[{inner}]" +def _relation_jinja( + name: str, + *, + raw: bool = False, + database: str | None = None, + schema: str | None = None, + glue_database: str | None = None, +) -> str: + """Render a relation reference for inline Jinja SQL. + + Normally a plain ``ref('name')`` — requires *name* to be a real dbt model in + the manifest. When ``raw=True``, builds a literal ``api.Relation.create(...)`` + instead, addressed by *database*/*schema* (defaulting to the current + ``--target``'s own database/schema) — for relations dbt doesn't know about, + e.g. an already-materialized table elsewhere. + + *glue_database*, when given, forces raw mode and rewrites the identifier to + the ``glue__{glue_database}__{name}`` naming convention that ``ol-dbt local + register`` uses for its DuckDB views (see ``local_dev.py``) — so callers can + pass the same bare table *name* for both a Glue-registered view and a real + dbt model instead of spelling out the full ``glue__...`` identifier. + """ + if glue_database: + raw = True + name = f"glue__{glue_database}__{name}" + if not raw: + return f"ref('{name}')" + db = f"'{database}'" if database else "target.database" + sch = f"'{schema}'" if schema else "target.schema" + return f"api.Relation.create(database={db}, schema={sch}, identifier='{name}')" + + def _compare_relations_sql( old: str, new: str, @@ -283,6 +392,14 @@ def _compare_relations_sql( exclude_columns: list[str], *, summarize: bool, + old_raw: bool = False, + new_raw: bool = False, + old_schema: str | None = None, + new_schema: str | None = None, + old_database: str | None = None, + new_database: str | None = None, + old_glue: str | None = None, + new_glue: str | None = None, ) -> str: """Build inline SQL calling ``audit_helper.compare_relations``. @@ -295,23 +412,58 @@ def _compare_relations_sql( pk_repr = f"'{primary_key[0]}'" if len(primary_key) == 1 else _jinja_list(primary_key) pk_arg = f", primary_key={pk_repr}" exclude_arg = f", exclude_columns={_jinja_list(exclude_columns)}" if exclude_columns else "" + a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema, glue_database=old_glue) + b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema, glue_database=new_glue) + # `}}` in an f-string is an escape for a single literal `}` (same as `{{` for + # `{`) — closing the Jinja `{{ ... }}` block from an f-string needs `}}}}`. return ( "{{ audit_helper.compare_relations(" - f"a_relation=ref('{old}'), b_relation=ref('{new}')" - f"{exclude_arg}{pk_arg}, summarize={'true' if summarize else 'false'}) }}" + f"a_relation={a_expr}, b_relation={b_expr}" + f"{exclude_arg}{pk_arg}, summarize={'true' if summarize else 'false'}) }}}}" ) -def _compare_column_sql(old: str, new: str, primary_key: list[str], column: str) -> str: +def _compare_column_sql( + old: str, + new: str, + primary_key: list[str], + column: str, + *, + old_raw: bool = False, + new_raw: bool = False, + old_schema: str | None = None, + new_schema: str | None = None, + old_database: str | None = None, + new_database: str | None = None, + old_glue: str | None = None, + new_glue: str | None = None, +) -> str: """Build inline SQL calling ``audit_helper.compare_column_values`` for one column.""" pk = f"'{primary_key[0]}'" if len(primary_key) == 1 else _jinja_list(primary_key) - # Model names are dbt identifiers rendered into a Jinja template compiled by - # dbt (not executed as a raw query here), so the S608 heuristic is a false positive. + a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema, glue_database=old_glue) + b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema, glue_database=new_glue) + # Only the primary key(s) + the one column being compared are needed here -- + # `compare_column_values` does a full outer join on a_query/b_query, and + # selecting every column (as this used to) forces that join to carry the + # whole row width for a single-column comparison, which is needless I/O on + # wide tables. + select_cols = ", ".join(dict.fromkeys([*primary_key, column])) + # a_query/b_query are built with {% set %}/{% endset %} so `a_expr`/`b_expr` + # (a ref()/api.Relation.create() call) are rendered by Jinja BEFORE + # compare_column_values ever sees the string. Embedding `{{ ... }}` directly + # inside a quoted string argument (the previous approach) is inert: Jinja + # treats string literals atomically and never re-parses braces inside them, + # so the literal text `{{ ref(...) }}` reached the database unrendered -- + # a real, pre-existing bug (every diff with a primary key hit this). return ( - "{{ audit_helper.compare_column_values(" # noqa: S608 - f"a_query=\"select * from {{{{ ref('{old}') }}}}\", " - f"b_query=\"select * from {{{{ ref('{new}') }}}}\", " - f"primary_key={pk}, column_to_compare='{column}') }}" + "{% set a_query %}" # noqa: S608 + f"select {select_cols} from {{{{ {a_expr} }}}}" + "{% endset %}" + "{% set b_query %}" + f"select {select_cols} from {{{{ {b_expr} }}}}" + "{% endset %}" + "{{ audit_helper.compare_column_values(" + f"a_query=a_query, b_query=b_query, primary_key={pk}, column_to_compare='{column}') }}}}" ) @@ -369,9 +521,53 @@ def _result_to_dict(result: DiffResult) -> dict[str, Any]: "sample_mismatches": result.sample_mismatches, "verdict": result.verdict.value, "notes": result.notes, + "old_label": result.old_label, + "new_label": result.new_label, } +def _format_sample_mismatches( + rows: list[dict[str, Any]], primary_key: list[str], old_label: str, new_label: str +) -> list[str]: + """Turn raw ``compare_relations(summarize=false)`` rows into readable per-key diffs. + + audit_helper tags each mismatching row with the full row content from + whichever side(s) it appears on (``in_a``/``in_b``) -- printed as raw JSON, + the 1-3 fields that actually differ are buried under the ~40 that don't. + This groups by primary key and reports only the differing fields, or + "only in " when a row exists on just one side. + """ + groups: dict[tuple[Any, ...], list[dict[str, Any]]] = {} + for row in rows: + key = tuple(row.get(k) for k in primary_key) + groups.setdefault(key, []).append(row) + + pk_desc = ", ".join(primary_key) + lines: list[str] = [] + for key, group_rows in groups.items(): + key_str = f"{pk_desc}={key[0]}" if len(key) == 1 else f"({pk_desc})={key}" + a_row = next((r for r in group_rows if r.get("in_a") and not r.get("in_b")), None) + b_row = next((r for r in group_rows if r.get("in_b") and not r.get("in_a")), None) + # A non-unique primary key can put more than 2 rows in one group -- flag + # it rather than silently comparing only the first pair and dropping the + # rest, which would understate how many rows actually differ. + extra = f" (+{len(group_rows) - 2} more rows in this key group, not shown)" if len(group_rows) > 2 else "" + if a_row and not b_row: + lines.append(f"{key_str}: only in {old_label}{extra}") + elif b_row and not a_row: + lines.append(f"{key_str}: only in {new_label}{extra}") + elif a_row and b_row: + diffs = [ + f"{field}: {a_row.get(field)!r} → {b_row.get(field)!r}" + for field in a_row + if field not in ("in_a", "in_b") and a_row.get(field) != b_row.get(field) + ] + lines.append( + f"{key_str}: " + ("; ".join(diffs) if diffs else "(no field differences in this sample)") + extra + ) + return lines + + def _print_json(result: DiffResult) -> None: print(json.dumps(_result_to_dict(result), indent=2)) @@ -384,22 +580,23 @@ def _print_text(result: DiffResult) -> None: }[result.verdict] console.print( f"\n[{color}]{result.verdict.value.upper()}[/] " - f"[bold]{result.old}[/] → [bold]{result.new}[/] ([dim]{result.target}[/])" + f"[bold]{result.old_label}[/] → [bold]{result.new_label}[/] ([dim]{result.target}[/])" ) recon = result.column_reconciliation if recon.diverged: if recon.only_in_old: - console.print(f" [yellow]Only in {result.old}:[/] {', '.join(recon.only_in_old)}") + console.print(f" [yellow]Only in {result.old_label}:[/] {', '.join(recon.only_in_old)}") if recon.only_in_new: - console.print(f" [yellow]Only in {result.new}:[/] {', '.join(recon.only_in_new)}") + console.print(f" [yellow]Only in {result.new_label}:[/] {', '.join(recon.only_in_new)}") console.print(f" [bold]Compared columns:[/] {len(recon.compared)}") rc = result.row_counts delta = rc.get("delta", 0) delta_str = f"[green]{delta}[/]" if delta == 0 else f"[red]{delta:+d}[/]" console.print( - f" [bold]Row counts:[/] {result.old}={rc.get('old', 0)} {result.new}={rc.get('new', 0)} Δ={delta_str}" + f" [bold]Row counts:[/] {result.old_label}={rc.get('old', 0)} " + f"{result.new_label}={rc.get('new', 0)} Δ={delta_str}" ) if result.column_mismatches: @@ -408,9 +605,20 @@ def _print_text(result: DiffResult) -> None: console.print(f" • {c.column}: {c.mismatch_rate:.2%} ({c.mismatched_rows} rows)") if result.sample_mismatches: - console.print(f" [bold]Sample mismatched rows[/] (first {len(result.sample_mismatches)}):") - for row in result.sample_mismatches: - console.print(f" {json.dumps(row, default=str)}") + if result.primary_key: + formatted = _format_sample_mismatches( + result.sample_mismatches, result.primary_key, result.old_label, result.new_label + ) + console.print(f" [bold]Sample mismatches[/] (first {len(formatted)} keys):") + for line in formatted: + console.print(f" • {line}") + else: + # No --primary-key means rows can't be reliably paired across sides, + # so fall back to the raw full-row dump rather than risk grouping + # unrelated rows together. + console.print(f" [bold]Sample mismatched rows[/] (first {len(result.sample_mismatches)}):") + for row in result.sample_mismatches: + console.print(f" {json.dumps(row, default=str)}") for note in result.notes: console.print(f" [dim]{note}[/]") @@ -432,9 +640,14 @@ def diff( Parameter(name=["--old"], help="Baseline model name (the 'before' relation)."), ], new: Annotated[ - str, - Parameter(name=["--new"], help="Candidate model name (the 'after' relation)."), - ], + str | None, + Parameter( + name=["--new"], + help="Candidate model name (the 'after' relation). Defaults to --old's value if omitted — " + "the common case is comparing the same table name via --old-raw/--old-glue against its " + "ref()-resolved build.", + ), + ] = None, dbt_dir_path: Annotated[ str | None, Parameter( @@ -483,11 +696,78 @@ def diff( bool, Parameter( name=["--auto-build"], - help="Run 'dbt build' on both models (on --target) before comparing, so both relations exist.", + help="Run 'dbt build' on both models (on --target) before comparing, so both relations exist. " + "Skipped for any side marked --old-raw/--new-raw, since raw relations aren't dbt models.", + ), + ] = False, + old_raw: Annotated[ + bool, + Parameter( + name=["--old-raw"], + help="Treat --old as a literal existing relation, not a dbt model — bypasses ref()/manifest " + "lookup. Use for a Glue-registered view (`ol-dbt local register`, a live view onto the real " + "production Iceberg table) or an already-materialized table dbt doesn't know about. Defaults " + "to the current --target's own database/schema; override with --old-database/--old-schema.", + ), + ] = False, + new_raw: Annotated[ + bool, + Parameter(name=["--new-raw"], help="Same as --old-raw, but for --new."), + ] = False, + old_schema: Annotated[ + str | None, + Parameter( + name=["--old-schema"], + help="Schema override for --old. Requires --old-raw (default: the current --target's schema).", + ), + ] = None, + new_schema: Annotated[ + str | None, + Parameter( + name=["--new-schema"], + help="Schema override for --new. Requires --new-raw (default: the current --target's schema).", + ), + ] = None, + old_database: Annotated[ + str | None, + Parameter( + name=["--old-database"], + help="Database/catalog override for --old. Requires --old-raw (default: the current --target's database).", + ), + ] = None, + new_database: Annotated[ + str | None, + Parameter( + name=["--new-database"], + help="Database/catalog override for --new. Requires --new-raw (default: the current --target's database).", + ), + ] = None, + old_glue: Annotated[ + str | None, + Parameter( + name=["--old-glue"], + help="Treat --old as a Glue-registered DuckDB view (`ol-dbt local register`) named " + "glue__{OLD_GLUE}__{old} — pass just the Glue database name and keep --old as the bare " + "table name. A live view onto the real production Iceberg table, not a copy. Implies " + "--old-raw; mutually exclusive with it.", + ), + ] = None, + new_glue: Annotated[ + str | None, + Parameter(name=["--new-glue"], help="Same as --old-glue, but for --new."), + ] = None, + refresh_glue: Annotated[ + bool, + Parameter( + name=["--refresh-glue"], + help="Run 'ol-dbt local register --all-layers' before comparing. Glue-registered views go " + "stale (HTTP 404 on their pinned S3 metadata) once production rebuilds the underlying table " + "-- this refreshes them first. Covers not just --old-glue/--new-glue but any upstream " + "Glue-registered dependency --auto-build's dbt build might hit via override_ref.sql.", ), ] = False, ) -> None: - """Diff two dbt model relations (same-engine row/column comparison). + r"""Diff two dbt model relations (same-engine row/column comparison). Reconciles column sets first (so a schema mismatch is reported clearly rather than as a raw SQL error), then runs a dbt_audit_helper comparison on the @@ -504,20 +784,93 @@ def diff( Build both sides first, then compare: ol-dbt diff --old m_old --new m_new --auto-build + Compare a migrated model's local build against the real, already-materialized + production table via its Glue-registered view. --new defaults to --old's value, + so only one bare table name is needed. --refresh-glue re-registers Glue views + first (`ol-dbt local register --all-layers`) instead of requiring it to have + been run separately -- worthwhile whenever the production table might have + rebuilt (and its old __dbt_tmp snapshot been cleaned up) since you last + registered it, which surfaces as an HTTP 404 on stale S3 metadata: + ol-dbt diff \\ + --old enrollment_detail_report --old-glue ol_warehouse_production_reporting \\ + --primary-key courserunenrollment_id --refresh-glue + + Compare two already-materialized copies of the same model across schemas + (e.g. your personal dev schema vs. real production) on one Trino target: + ol-dbt diff --target dev_production \\ + --old enrollment_detail_report --old-raw --old-schema ol_warehouse_production_reporting \\ + --new enrollment_detail_report --new-raw --new-schema ol_warehouse_production_rlougee_reporting \\ + --primary-key courserunenrollment_id + """ + new = new or old primary_key_list = list(primary_key) exclude_list = list(exclude_columns) notes: list[str] = [] + if old_raw and old_glue: + err_console.print("[red]Error:[/] --old-raw and --old-glue are mutually exclusive.") + sys.exit(1) + if new_raw and new_glue: + err_console.print("[red]Error:[/] --new-raw and --new-glue are mutually exclusive.") + sys.exit(1) + + # --old-glue/--new-glue are a --*-raw variant (see _relation_jinja), so schema/ + # database overrides are valid alongside either. + old_is_raw = old_raw or bool(old_glue) + new_is_raw = new_raw or bool(new_glue) + + if (old_schema or old_database) and not old_is_raw: + err_console.print("[red]Error:[/] --old-schema/--old-database require --old-raw or --old-glue.") + sys.exit(1) + if (new_schema or new_database) and not new_is_raw: + err_console.print("[red]Error:[/] --new-schema/--new-database require --new-raw or --new-glue.") + sys.exit(1) + # Validate every value interpolated into inline Jinja SQL before use. try: _validate_identifiers("model name", [old, new]) _validate_identifiers("primary key", primary_key_list) _validate_identifiers("excluded column", exclude_list) + _validate_identifiers( + "schema/database override", + [v for v in (old_schema, new_schema, old_database, new_database) if v], + ) + _validate_identifiers("glue database", [v for v in (old_glue, new_glue) if v]) except InvalidIdentifierError as exc: err_console.print(f"[red]Error:[/] {exc}") sys.exit(1) + if old_glue: + notes.append( + f"--old-glue: '{old}' resolved as Glue-registered view " + f"'glue__{old_glue}__{old}' (database={old_database or 'target.database'}, " + f"schema={old_schema or 'target.schema'})." + ) + elif old_raw: + notes.append( + f"--old-raw: '{old}' resolved as a literal relation " + f"(database={old_database or 'target.database'}, schema={old_schema or 'target.schema'})." + ) + if new_glue: + notes.append( + f"--new-glue: '{new}' resolved as Glue-registered view " + f"'glue__{new_glue}__{new}' (database={new_database or 'target.database'}, " + f"schema={new_schema or 'target.schema'})." + ) + elif new_raw: + notes.append( + f"--new-raw: '{new}' resolved as a literal relation " + f"(database={new_database or 'target.database'}, schema={new_schema or 'target.schema'})." + ) + + # Text-output labels disambiguating which side is which when --old/new-raw or + # --old/new-glue make the bare `old`/`new` names identical or otherwise + # ambiguous (e.g. both "enrollment_detail_report"). Unannotated for a plain + # ref()-vs-ref() diff, where `old`/`new` are already unambiguous. + old_label = f"{old} (glue:{old_glue})" if old_glue else (f"{old} (raw)" if old_raw else old) + new_label = f"{new} (glue:{new_glue})" if new_glue else (f"{new} (raw)" if new_raw else new) + # Resolve dbt project directory (shared preamble with impact/validate). if dbt_dir_path: dbt_dir = Path(dbt_dir_path).resolve() @@ -532,6 +885,61 @@ def diff( err_console.print(" Use --dbt-dir to specify the path.") sys.exit(1) + # Fail fast with an actionable message rather than letting dbt's raw + # "N package(s) specified... only M installed" Compilation Error (surfaced + # through 500 chars of dbt log noise) reach the user. + if not (dbt_dir / "dbt_packages" / "audit_helper").exists(): + err_console.print(f"[red]Error:[/] the 'audit_helper' dbt package is not installed in {dbt_dir}.") + err_console.print(f" Run: cd {dbt_dir} && dbt deps") + sys.exit(1) + + # Refresh Glue-registered views BEFORE anything below reads one -- both the + # materialization just below and --auto-build's `dbt build` (whose compiled + # SQL can hit other Glue-registered views transitively, via override_ref.sql) + # can fail with a stale-metadata 404 once production rebuilds the underlying + # table out from under a previously-registered view. + if refresh_glue: + if output_format == "text": + console.print("[dim]Running: ol-dbt local register --all-layers ...[/]") + try: + subprocess.run( # noqa: S603, S607 + ["ol-dbt", "local", "register", "--all-layers"], + capture_output=True, + text=True, + check=True, + ) + except subprocess.CalledProcessError as exc: + detail = (exc.stderr or exc.stdout or str(exc)).strip() + err_console.print(f"[red]Error:[/] ol-dbt local register --all-layers failed: {detail[-500:]}") + sys.exit(1) + except FileNotFoundError: + err_console.print("[red]Error:[/] 'ol-dbt' command not found; ensure it is on PATH.") + sys.exit(1) + + # --old-glue/--new-glue relations are backed by iceberg_scan() views, which + # DuckDB's Iceberg extension can't serialize into audit_helper's UNION + # ALL/EXCEPT comparison SQL (see _materialize_glue_relation). Swap them for a + # materialized copy before any column resolution or comparison runs. These + # `*_query` variables are the ones actually used to build SQL below; + # `old`/`new`/`old_label`/`new_label` are left untouched so notes/output keep + # showing the original Glue identifier the user asked to compare. + old_query, old_query_glue, old_query_database, old_query_schema = old, old_glue, old_database, old_schema + new_query, new_query_glue, new_query_database, new_query_schema = new, new_glue, new_database, new_schema + if old_glue: + try: + old_query = _materialize_glue_relation(old, old_glue, dbt_dir, target) + except RuntimeError as exc: + err_console.print(f"[red]Error:[/] failed to materialize --old-glue relation: {exc}") + sys.exit(1) + old_query_glue, old_query_database, old_query_schema = None, None, None + if new_glue: + try: + new_query = _materialize_glue_relation(new, new_glue, dbt_dir, target) + except RuntimeError as exc: + err_console.print(f"[red]Error:[/] failed to materialize --new-glue relation: {exc}") + sys.exit(1) + new_query_glue, new_query_database, new_query_schema = None, None, None + models_dir = dbt_dir / "models" # Build the metadata sources used for column reconciliation. @@ -553,13 +961,44 @@ def diff( continue # --- Column reconciliation (before any comparison) --- - old_cols = _resolve_columns(old, sql_models_by_name, manifest, yaml_registry) - new_cols = _resolve_columns(new, sql_models_by_name, manifest, yaml_registry) + old_raw_error: str | None = None + new_raw_error: str | None = None + if old_is_raw: + old_cols, old_raw_error = _resolve_raw_columns( + old_query, + dbt_dir, + target, + database=old_query_database, + schema=old_query_schema, + glue_database=old_query_glue, + ) + else: + old_cols = _resolve_columns(old, sql_models_by_name, manifest, yaml_registry) + if new_is_raw: + new_cols, new_raw_error = _resolve_raw_columns( + new_query, + dbt_dir, + target, + database=new_query_database, + schema=new_query_schema, + glue_database=new_query_glue, + ) + else: + new_cols = _resolve_columns(new, sql_models_by_name, manifest, yaml_registry) + unresolved_note = "Column set for '{}' could not be resolved (unparsed / not in manifest); reconciliation skipped." if not old_cols: - notes.append(unresolved_note.format(old)) + notes.append( + f"Column set for '{old}' could not be resolved: {old_raw_error}" + if old_raw_error + else unresolved_note.format(old) + ) if not new_cols: - notes.append(unresolved_note.format(new)) + notes.append( + f"Column set for '{new}' could not be resolved: {new_raw_error}" + if new_raw_error + else unresolved_note.format(new) + ) recon = reconcile_columns(old_cols, new_cols, set(exclude_list)) # When a column set can't be resolved statically, the schema-divergence gate @@ -580,22 +1019,28 @@ def diff( "(or point --dbt-dir at a compiled project) for full schema validation." ) - # Optionally build both relations first. + # Optionally build both relations first. Raw relations aren't dbt models — + # they're assumed to already exist (a Glue view, an already-materialized + # table) — so they're excluded from --select rather than passed to dbt build. if auto_build: - # Single space-joined --select value, consistent with ol-dbt run/impact. - build_cmd = ["dbt", "build", "--target", target, "--select", f"{old} {new}"] - if output_format == "text": - console.print(f"[dim]Running: {' '.join(build_cmd)} ...[/]") - try: - subprocess.run(build_cmd, cwd=str(dbt_dir), capture_output=True, text=True, check=True) # noqa: S603, S607 - except subprocess.CalledProcessError as exc: - # dbt often logs useful detail to stdout, not stderr — prefer either. - detail = (exc.stderr or exc.stdout or str(exc)).strip() - err_console.print(f"[red]Error:[/] dbt build failed: {detail[-500:]}") - sys.exit(1) - except FileNotFoundError: - err_console.print("[red]Error:[/] 'dbt' command not found; install dbt and ensure it is on PATH.") - sys.exit(1) + build_targets = [name for name, raw in ((old, old_is_raw), (new, new_is_raw)) if not raw] + if not build_targets: + notes.append("--auto-build skipped: both --old and --new are raw relations (nothing to build).") + else: + # Single space-joined --select value, consistent with ol-dbt run/impact. + build_cmd = ["dbt", "build", "--target", target, "--select", " ".join(build_targets)] + if output_format == "text": + console.print(f"[dim]Running: {' '.join(build_cmd)} ...[/]") + try: + subprocess.run(build_cmd, cwd=str(dbt_dir), capture_output=True, text=True, check=True) # noqa: S603, S607 + except subprocess.CalledProcessError as exc: + # dbt often logs useful detail to stdout, not stderr — prefer either. + detail = (exc.stderr or exc.stdout or str(exc)).strip() + err_console.print(f"[red]Error:[/] dbt build failed: {detail[-500:]}") + sys.exit(1) + except FileNotFoundError: + err_console.print("[red]Error:[/] 'dbt' command not found; install dbt and ensure it is on PATH.") + sys.exit(1) # --- Comparison --- row_counts: dict[str, int] = {"old": 0, "new": 0, "delta": 0} @@ -619,6 +1064,8 @@ def diff( column_mismatches=column_mismatches, sample_mismatches=sample_mismatches, verdict=Verdict.SCHEMA_DIVERGENCE, + old_label=old_label, + new_label=new_label, notes=notes, ), output_format, @@ -626,7 +1073,21 @@ def diff( try: summary_rows = _run_dbt_show( - _compare_relations_sql(old, new, primary_key_list, exclude_list, summarize=True), + _compare_relations_sql( + old_query, + new_query, + primary_key_list, + exclude_list, + summarize=True, + old_raw=old_is_raw, + new_raw=new_is_raw, + old_schema=old_query_schema, + new_schema=new_query_schema, + old_database=old_query_database, + new_database=new_query_database, + old_glue=old_query_glue, + new_glue=new_query_glue, + ), dbt_dir, target, limit=1000, @@ -635,7 +1096,21 @@ def diff( if mismatched_rows: sample_mismatches = _run_dbt_show( - _compare_relations_sql(old, new, primary_key_list, exclude_list, summarize=False), + _compare_relations_sql( + old_query, + new_query, + primary_key_list, + exclude_list, + summarize=False, + old_raw=old_is_raw, + new_raw=new_is_raw, + old_schema=old_query_schema, + new_schema=new_query_schema, + old_database=old_query_database, + new_database=new_query_database, + old_glue=old_query_glue, + new_glue=new_query_glue, + ), dbt_dir, target, limit=limit, @@ -648,7 +1123,22 @@ def diff( notes.append(f"Per-column comparison capped at {_MAX_PER_COLUMN_COMPARISONS} of {len(cols)} columns.") cols = cols[:_MAX_PER_COLUMN_COMPARISONS] for col in cols: - mismatch = _compare_single_column(old, new, primary_key_list, col, dbt_dir, target) + mismatch = _compare_single_column( + old_query, + new_query, + primary_key_list, + col, + dbt_dir, + target, + old_raw=old_is_raw, + new_raw=new_is_raw, + old_schema=old_query_schema, + new_schema=new_query_schema, + old_database=old_query_database, + new_database=new_query_database, + old_glue=old_query_glue, + new_glue=new_query_glue, + ) if mismatch is not None and mismatch.mismatched_rows > 0: column_mismatches.append(mismatch) elif not primary_key_list: @@ -675,6 +1165,8 @@ def diff( column_mismatches=column_mismatches, sample_mismatches=sample_mismatches, verdict=verdict, + old_label=old_label, + new_label=new_label, notes=notes, ), output_format, @@ -698,6 +1190,15 @@ def _compare_single_column( column: str, dbt_dir: Path, target: str, + *, + old_raw: bool = False, + new_raw: bool = False, + old_schema: str | None = None, + new_schema: str | None = None, + old_database: str | None = None, + new_database: str | None = None, + old_glue: str | None = None, + new_glue: str | None = None, ) -> ColumnMismatch | None: """Run a per-column value comparison, returning its mismatch rate. @@ -705,7 +1206,20 @@ def _compare_single_column( the mismatch rate is the share of rows not in a "✅" perfect-match bucket. """ rows = _run_dbt_show( - _compare_column_sql(old, new, primary_key, column), + _compare_column_sql( + old, + new, + primary_key, + column, + old_raw=old_raw, + new_raw=new_raw, + old_schema=old_schema, + new_schema=new_schema, + old_database=old_database, + new_database=new_database, + old_glue=old_glue, + new_glue=new_glue, + ), dbt_dir, target, limit=1000, diff --git a/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py b/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py index 7d3b2526c..84c554c8c 100644 --- a/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py +++ b/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py @@ -9,6 +9,7 @@ from __future__ import annotations import os +import subprocess import sys from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path @@ -675,6 +676,74 @@ def list_sources( _show_registry(duckdb_path) +@local_app.command +def snapshot( + model: Annotated[str, cyclopts.Parameter(help="Model name to snapshot (its current build, via ref()).")], + as_name: Annotated[ + str, + cyclopts.Parameter(name=["--as"], help="Name for the snapshot table, created in the target's own schema."), + ], + target: Annotated[ + str, + cyclopts.Parameter(name=["--target", "-t"], help="dbt target to read/write on (default: dev_local)."), + ] = "dev_local", + dbt_dir_path: Annotated[ + str | None, + cyclopts.Parameter( + name=["--dbt-dir", "-d"], + help="Path to dbt project root (contains dbt_project.yml). Defaults to src/ol_dbt relative to repo root.", + ), + ] = None, +) -> None: + """Copy a model's current build into a plain table under a new name. + + For before/after comparisons of the SAME model (a code change, not a + migration to a differently-named model): build it, snapshot the build with + this command, make your change and rebuild, then compare with + `ol-dbt diff --old --old-raw --new --primary-key ...`. + Neither this command nor that diff touches Glue/production, so any + difference it finds reflects the code change alone, not upstream data + drift from asynchronous production rebuilds. + """ + from ol_dbt_cli.commands.diff import InvalidIdentifierError, _validate_identifiers # noqa: PLC0415 + from ol_dbt_cli.commands.run import _find_dbt_dir # noqa: PLC0415 + + try: + _validate_identifiers("model/snapshot name", [model, as_name]) + except InvalidIdentifierError as exc: + print(f"Error: {exc}") + sys.exit(1) + + try: + dbt_dir = _find_dbt_dir(dbt_dir_path) + except RuntimeError as exc: + print(f"Error: {exc}") + sys.exit(1) + + # Rendered into a Jinja template compiled by dbt (not executed as a raw query + # here), so the S608 heuristic is a false positive -- model/as_name were + # already validated as plain identifiers above. + # `}}` in an f-string is an escape for a single literal `}` (same as `{{` for + # `{`) -- each Jinja `{{ ... }}` block needs `{{{{`/`}}}}` from an f-string. + dst_expr = f"api.Relation.create(database=target.database, schema=target.schema, identifier='{as_name}')" + src_expr = f"ref('{model}')" + ctas_sql = f"create or replace table {{{{ {dst_expr} }}}} as select * from {{{{ {src_expr} }}}}" # noqa: S608 + cmd = ["dbt", "show", "--inline", ctas_sql, "--target", target, "--output", "json", "--limit", "-1"] + print(f"Running: {' '.join(cmd)}") + try: + subprocess.run(cmd, cwd=str(dbt_dir), capture_output=True, text=True, check=True) # noqa: S603 + except subprocess.CalledProcessError as exc: + detail = (exc.stderr or exc.stdout or str(exc)).strip() + print(f"Error: dbt show failed: {detail[-500:]}") + sys.exit(1) + except FileNotFoundError: + print("Error: 'dbt' command not found; install dbt and ensure it is on PATH.") + sys.exit(1) + + print(f"✅ Snapshotted '{model}' as '{as_name}' (target={target}).") + print(f" Compare with: ol-dbt diff --target {target} --old {as_name} --old-raw --new {model} --primary-key ") + + @local_app.command def cleanup_local( duckdb_path: Annotated[ diff --git a/src/ol_dbt_cli/tests/test_diff.py b/src/ol_dbt_cli/tests/test_diff.py index eeff863a7..4259f4253 100644 --- a/src/ol_dbt_cli/tests/test_diff.py +++ b/src/ol_dbt_cli/tests/test_diff.py @@ -10,9 +10,13 @@ from ol_dbt_cli.commands import diff as diff_mod from ol_dbt_cli.commands.diff import ( Verdict, + _compare_column_sql, _compare_relations_sql, _extract_show_rows, + _format_sample_mismatches, _jinja_list, + _relation_jinja, + _resolve_raw_columns, _summarize_relations, diff, reconcile_columns, @@ -136,6 +140,34 @@ def test_rows_on_both_sides(self) -> None: assert mismatched == 12 +class TestFormatSampleMismatches: + def test_value_diff_shows_only_differing_fields(self) -> None: + # a_row/b_row share id=1 but differ only in "name" -- "other" must NOT + # appear in the output even though it's present in both raw rows. + rows = [ + {"id": 1, "name": "old", "other": "same", "in_a": True, "in_b": False}, + {"id": 1, "name": "new", "other": "same", "in_a": False, "in_b": True}, + ] + lines = _format_sample_mismatches(rows, ["id"], "old_side", "new_side") + assert lines == ["id=1: name: 'old' → 'new'"] + + def test_row_present_only_on_one_side(self) -> None: + rows = [{"id": 2, "name": "x", "in_a": True, "in_b": False}] + lines = _format_sample_mismatches(rows, ["id"], "old_side", "new_side") + assert lines == ["id=2: only in old_side"] + + def test_flags_when_key_group_has_more_than_two_rows(self) -> None: + # A non-unique primary key can put >2 rows in one group -- must flag + # the extras rather than silently comparing only the first pair. + rows = [ + {"id": 3, "name": "a", "in_a": True, "in_b": False}, + {"id": 3, "name": "b", "in_a": True, "in_b": False}, + {"id": 3, "name": "c", "in_a": True, "in_b": False}, + ] + lines = _format_sample_mismatches(rows, ["id"], "old_side", "new_side") + assert lines == ["id=3: only in old_side (+1 more rows in this key group, not shown)"] + + class TestSqlBuilders: def test_jinja_list(self) -> None: assert _jinja_list(["a", "b"]) == "['a', 'b']" @@ -158,13 +190,122 @@ def test_compare_relations_no_pk(self) -> None: assert "primary_key" not in sql -def _make_project(tmp_path: Path, old_sql: str, new_sql: str) -> Path: - """Create a minimal dbt project dir with two leaf models.""" +class TestRelationJinja: + def test_non_raw_uses_ref(self) -> None: + assert _relation_jinja("dim_user") == "ref('dim_user')" + + def test_raw_defaults_to_target_database_and_schema(self) -> None: + expr = _relation_jinja("glue__ol_warehouse_production_reporting__enrollment_detail_report", raw=True) + assert expr == ( + "api.Relation.create(database=target.database, schema=target.schema, " + "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report')" + ) + + def test_raw_with_explicit_database_and_schema(self) -> None: + expr = _relation_jinja( + "enrollment_detail_report", + raw=True, + database="ol_data_lake_production", + schema="ol_warehouse_production_reporting", + ) + assert expr == ( + "api.Relation.create(database='ol_data_lake_production', " + "schema='ol_warehouse_production_reporting', identifier='enrollment_detail_report')" + ) + + def test_compare_relations_sql_with_old_raw(self) -> None: + sql = _compare_relations_sql( + "glue__x__y", + "new_m", + ["id"], + [], + summarize=True, + old_raw=True, + old_schema="main", + ) + assert "ref('glue__x__y')" not in sql + assert "api.Relation.create(database=target.database, schema='main', identifier='glue__x__y')" in sql + assert "ref('new_m')" in sql + + def test_compare_column_sql_with_both_raw(self) -> None: + sql = _compare_column_sql( + "old_tbl", + "new_tbl", + ["id"], + "some_col", + old_raw=True, + old_database="db_a", + old_schema="schema_a", + new_raw=True, + new_database="db_b", + new_schema="schema_b", + ) + assert "api.Relation.create(database='db_a', schema='schema_a', identifier='old_tbl')" in sql + assert "api.Relation.create(database='db_b', schema='schema_b', identifier='new_tbl')" in sql + assert "ref(" not in sql + + def test_compare_column_sql_narrows_to_primary_key_and_column(self) -> None: + # Narrows to primary key + compared column instead of `select *`, since + # compare_column_values' full outer join doesn't need the rest of the row. + sql = _compare_column_sql("old_m", "new_m", ["id"], "some_col") + assert "select id, some_col from" in sql + assert "select *" not in sql + + def test_glue_database_rewrites_identifier_and_forces_raw(self) -> None: + expr = _relation_jinja("enrollment_detail_report", glue_database="ol_warehouse_production_reporting") + assert expr == ( + "api.Relation.create(database=target.database, schema=target.schema, " + "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report')" + ) + + def test_compare_relations_sql_with_old_glue(self) -> None: + sql = _compare_relations_sql( + "enrollment_detail_report", + "enrollment_detail_report", + ["id"], + [], + summarize=True, + old_glue="ol_warehouse_production_reporting", + ) + assert "ref('enrollment_detail_report')" in sql # --new side unaffected + assert "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in sql + assert "ref('glue__" not in sql + + +class TestResolveRawColumns: + def test_returns_lowercased_keys_from_sampled_row(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: [{"Id": 1, "NAME": "x"}]) + cols, error = _resolve_raw_columns("some_view", tmp_path, "dev_local") + assert cols == {"id", "name"} + assert error is None + + def test_empty_result_returns_empty_set_no_error(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: []) + cols, error = _resolve_raw_columns("some_view", tmp_path, "dev_local") + assert cols == set() + assert error is None + + def test_dbt_failure_returns_empty_set_and_surfaces_error( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + def boom(*a: Any, **k: Any) -> list[dict[str, Any]]: + msg = "dbt show failed: relation glue__x__y not found" + raise RuntimeError(msg) + + monkeypatch.setattr(diff_mod, "_run_dbt_show", boom) + cols, error = _resolve_raw_columns("missing_view", tmp_path, "dev_local") + assert cols == set() + assert error == "dbt show failed: relation glue__x__y not found" + + +def _make_project(tmp_path: Path, old_sql: str, new_sql: str, *, new_name: str = "m_new") -> Path: + """Create a minimal dbt project dir with two leaf models and audit_helper "installed".""" dbt_dir = tmp_path / "ol_dbt" (dbt_dir / "models").mkdir(parents=True) + (dbt_dir / "dbt_packages" / "audit_helper").mkdir(parents=True) (dbt_dir / "dbt_project.yml").write_text("name: test\nprofile: test\n") (dbt_dir / "models" / "m_old.sql").write_text(old_sql) - (dbt_dir / "models" / "m_new.sql").write_text(new_sql) + (dbt_dir / "models" / f"{new_name}.sql").write_text(new_sql) return dbt_dir @@ -296,6 +437,8 @@ def test_json_output_schema( "row_counts", "column_mismatches", "sample_mismatches", + "old_label", + "new_label", ): assert key in payload assert set(payload["column_reconciliation"]) == {"only_in_old", "only_in_new", "compared"} @@ -346,3 +489,288 @@ def raise_runtime(*a: Any, **k: Any) -> list[dict[str, Any]]: with pytest.raises(SystemExit) as exc: diff(old="m_old", new="m_new", dbt_dir_path=str(dbt_dir)) assert exc.value.code == 1 + + +class TestDiffCommandRaw: + def test_old_schema_without_old_raw_exits_1(self, tmp_path: Path) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + with pytest.raises(SystemExit) as exc: + diff(old="m_old", new="m_new", old_schema="main", dbt_dir_path=str(dbt_dir)) + assert exc.value.code == 1 + + def test_new_database_without_new_raw_exits_1(self, tmp_path: Path) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + with pytest.raises(SystemExit) as exc: + diff(old="m_old", new="m_new", new_database="db_b", dbt_dir_path=str(dbt_dir)) + assert exc.value.code == 1 + + def test_injected_schema_override_rejected(self, tmp_path: Path) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + with pytest.raises(SystemExit) as exc: + diff(old="m_old", new="m_new", old_raw=True, old_schema="a; drop table x", dbt_dir_path=str(dbt_dir)) + assert exc.value.code == 1 + + def test_old_raw_match_exits_0_and_notes_resolution( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + # "old" is a raw relation (e.g. a Glue view) that isn't a dbt model at all — + # it must not go through static column resolution or ref(). + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + seen_inline: list[str] = [] + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + seen_inline.append(inline_sql) + if "audit_helper" not in inline_sql: + return [{"id": 1, "name": "x"}] # raw column sample + return [{"in_a": True, "in_b": True, "count": 10}] + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + diff( + old="glue__ol_warehouse_production_reporting__enrollment_detail_report", + new="m_new", + old_raw=True, + old_schema="main", + dbt_dir_path=str(dbt_dir), + ) + err = capsys.readouterr().err + assert "schema-divergence" not in err + # ref() must never be called for the raw side. + assert not any("ref('glue__" in sql for sql in seen_inline) + assert any("api.Relation.create(database=target.database, schema='main'" in sql for sql in seen_inline) + + def test_auto_build_skips_raw_sides(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + if "compare_relations" in inline_sql: + return [{"in_a": True, "in_b": True, "count": 1}] + return [{"id": 1, "name": "x"}] # raw column sample, matching m_new's parsed columns + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + build_calls: list[list[str]] = [] + + def fake_run(cmd: list[str], **kwargs: Any) -> Any: + build_calls.append(cmd) + import subprocess + + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr(diff_mod.subprocess, "run", fake_run) + diff( + old="glue__x__y", + new="m_new", + old_raw=True, + auto_build=True, + dbt_dir_path=str(dbt_dir), + ) + assert len(build_calls) == 1 + select_idx = build_calls[0].index("--select") + 1 + assert build_calls[0][select_idx] == "m_new" + + def test_auto_build_skipped_entirely_when_both_raw(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + monkeypatch.setattr( + diff_mod, + "_run_dbt_show", + lambda *a, **k: [{"in_a": True, "in_b": True, "count": 1}], + ) + + def boom(*a: Any, **k: Any) -> Any: + msg = "dbt build must not run when both sides are raw" + raise AssertionError(msg) + + monkeypatch.setattr(diff_mod.subprocess, "run", boom) + diff( + old="glue__x__y", + new="glue__a__b", + old_raw=True, + new_raw=True, + auto_build=True, + dbt_dir_path=str(dbt_dir), + ) + + def test_refresh_glue_registers_all_layers_before_comparing( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # --refresh-glue must run before any Glue-registered view is read -- + # both the --old-glue/--new-glue materialization and (for --auto-build) + # dbt build's own upstream deps can 404 on stale S3 metadata otherwise. + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: [{"in_a": True, "in_b": True, "count": 1}]) + calls: list[list[str]] = [] + + def fake_run(cmd: list[str], **kwargs: Any) -> Any: + calls.append(cmd) + import subprocess + + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr(diff_mod.subprocess, "run", fake_run) + diff(old="m_old", new="m_new", refresh_glue=True, dbt_dir_path=str(dbt_dir)) + assert calls[0] == ["ol-dbt", "local", "register", "--all-layers"] + + +class TestDiffCommandGlue: + def test_old_glue_and_old_raw_together_exits_1(self, tmp_path: Path) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + with pytest.raises(SystemExit) as exc: + diff( + old="m_old", + new="m_new", + old_raw=True, + old_glue="ol_warehouse_production_reporting", + dbt_dir_path=str(dbt_dir), + ) + assert exc.value.code == 1 + + def test_invalid_glue_database_rejected(self, tmp_path: Path) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + with pytest.raises(SystemExit) as exc: + diff(old="m_old", new="m_new", old_glue="a; drop table x", dbt_dir_path=str(dbt_dir)) + assert exc.value.code == 1 + + def test_old_glue_keeps_bare_name_and_rewrites_identifier( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + # --old and --new both stay "enrollment_detail_report" as the underlying + # `old`/`new` identifiers — only the SQL identifier and the printed label + # should carry the glue__/(glue:...) annotation. + dbt_dir = _make_project( + tmp_path, + "select 1 as id, 'x' as name", + "select 1 as id, 'x' as name", + new_name="enrollment_detail_report", + ) + seen_inline: list[str] = [] + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + seen_inline.append(inline_sql) + if "audit_helper" not in inline_sql: + return [{"id": 1, "name": "x"}] # raw column sample + return [{"in_a": True, "in_b": True, "count": 10}] + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + diff( + old="enrollment_detail_report", + new="enrollment_detail_report", + old_glue="ol_warehouse_production_reporting", + dbt_dir_path=str(dbt_dir), + ) + out = " ".join(capsys.readouterr().out.split()) # rich line-wraps the long header + assert "enrollment_detail_report (glue:ol_warehouse_production_reporting) → enrollment_detail_report" in out + assert not any("ref('glue__" in sql for sql in seen_inline) + assert any( + "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in sql + for sql in seen_inline + ) + assert any("ref('enrollment_detail_report')" in sql for sql in seen_inline) + + def test_old_glue_materializes_scratch_table_before_comparing( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # DuckDB's Iceberg extension can't run audit_helper's UNION ALL/EXCEPT SQL + # against a live glue__ view (IcebergScan serialization not implemented), + # so the glue side must be copied into a real scratch table first, and + # every downstream comparison query must reference that scratch table -- + # never the live glue__ identifier -- or the same crash resurfaces. + dbt_dir = _make_project( + tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name", new_name="enrollment_detail_report" + ) + seen_inline: list[str] = [] + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + seen_inline.append(inline_sql) + if "audit_helper" not in inline_sql: + return [{"id": 1, "name": "x"}] + return [{"in_a": True, "in_b": True, "count": 10}] + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + diff( + old="enrollment_detail_report", + new="enrollment_detail_report", + old_glue="ol_warehouse_production_reporting", + dbt_dir_path=str(dbt_dir), + ) + ctas_calls = [sql for sql in seen_inline if "create or replace table" in sql] + assert len(ctas_calls) == 1 + assert ( + "identifier='_ol_dbt_diff_scratch__ol_warehouse_production_reporting__enrollment_detail_report'" + in (ctas_calls[0]) + ) + assert "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in ctas_calls[0] + + compare_calls = [sql for sql in seen_inline if "audit_helper" in sql] + assert compare_calls # summarize + (no mismatches -> no sample) at least one + assert not any("glue__" in sql for sql in compare_calls) + assert all( + "identifier='_ol_dbt_diff_scratch__ol_warehouse_production_reporting__enrollment_detail_report'" in sql + for sql in compare_calls + ) + + def test_old_glue_sampling_failure_surfaces_real_dbt_error( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + # If the Glue view doesn't exist (wrong glue database, not registered yet, + # ...), materializing it (which now runs before column resolution, to work + # around DuckDB's IcebergScan/UNION-ALL limitation) fails first — the real + # dbt error must be surfaced, not the generic "unparsed / not in manifest" + # note written for the static-parse path. + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + msg = "dbt show failed: Relation glue__bad_db__m_old does not exist" + raise RuntimeError(msg) + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + with pytest.raises(SystemExit) as exc: + diff(old="m_old", new="m_new", old_glue="bad_db", dbt_dir_path=str(dbt_dir)) + assert exc.value.code == 1 + err = " ".join(capsys.readouterr().err.split()) # rich line-wraps long notes + assert "failed to materialize --old-glue relation" in err + assert "Relation glue__bad_db__m_old does not exist" in err + assert "not in manifest" not in err + + +class TestDiffCommandLabels: + def test_plain_diff_labels_are_unannotated( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + # A plain ref()-vs-ref() diff (no --*-raw/--*-glue) is already unambiguous + # — the header must not grow a "(raw)"/"(glue:...)" annotation. + dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") + monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: [{"in_a": True, "in_b": True, "count": 1}]) + diff(old="m_old", new="m_new", dbt_dir_path=str(dbt_dir)) + out = capsys.readouterr().out + assert "m_old → m_new" in out + assert "(raw)" not in out + assert "(glue:" not in out + + def test_old_raw_label_annotated_with_raw( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + if "audit_helper" not in inline_sql: + return [{"id": 1, "name": "x"}] + return [{"in_a": True, "in_b": True, "count": 1}] + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + diff(old="glue__x__y", new="m_new", old_raw=True, dbt_dir_path=str(dbt_dir)) + out = capsys.readouterr().out + assert "glue__x__y (raw) → m_new" in out + + def test_old_glue_label_annotated_with_glue_database( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") + + def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: + if "audit_helper" not in inline_sql: + return [{"id": 1, "name": "x"}] + return [{"in_a": True, "in_b": True, "count": 1}] + + monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) + diff(old="m_old", new="m_new", old_glue="ol_warehouse_production_reporting", dbt_dir_path=str(dbt_dir)) + out = capsys.readouterr().out + assert "m_old (glue:ol_warehouse_production_reporting) → m_new" in out diff --git a/src/ol_dbt_cli/tests/test_local_dev.py b/src/ol_dbt_cli/tests/test_local_dev.py index 83bb173a4..36d1a218b 100644 --- a/src/ol_dbt_cli/tests/test_local_dev.py +++ b/src/ol_dbt_cli/tests/test_local_dev.py @@ -11,9 +11,39 @@ PROTECTED_SCHEMAS, _register_single_table, _validate_schema_safety, + snapshot, ) +class TestSnapshot: + def test_runs_ctas_with_validated_identifiers(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + import subprocess + + calls: list[list[str]] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + calls.append(cmd) + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + snapshot( + "enrollment_detail_report", + as_name="enrollment_detail_report_baseline", + dbt_dir_path=str(tmp_path), + ) + assert len(calls) == 1 + inline_idx = calls[0].index("--inline") + 1 + inline_sql = calls[0][inline_idx] + assert "identifier='enrollment_detail_report_baseline'" in inline_sql + assert "ref('enrollment_detail_report')" in inline_sql + assert "--limit" in calls[0] + assert calls[0][calls[0].index("--limit") + 1] == "-1" + + def test_rejects_invalid_identifier(self, tmp_path: Path) -> None: + with pytest.raises(SystemExit): + snapshot("m; drop table x", as_name="baseline", dbt_dir_path=str(tmp_path)) + + class TestValidateSchemaSafety: """Tests for _validate_schema_safety — guards against dropping production schemas.""" From a0da3073ffe16a6d8b4a8ca3c05956b8139b862d Mon Sep 17 00:00:00 2001 From: rlougee Date: Wed, 22 Jul 2026 14:16:25 -0400 Subject: [PATCH 2/3] feat(diff): remove --old-glue/--new-glue parameters and update documentation for ol-dbt diff --- src/ol_dbt/README.md | 53 ++++ src/ol_dbt_cli/ol_dbt_cli/commands/diff.py | 306 +++++---------------- src/ol_dbt_cli/tests/test_diff.py | 181 +----------- 3 files changed, 123 insertions(+), 417 deletions(-) diff --git a/src/ol_dbt/README.md b/src/ol_dbt/README.md index b8a86ccaa..b5e8655ed 100644 --- a/src/ol_dbt/README.md +++ b/src/ol_dbt/README.md @@ -103,6 +103,59 @@ ol-dbt run test The `.dbt-state/` directory is gitignored and local to your machine. +### Verifying a Change with `ol-dbt diff` and `ol-dbt local snapshot` + +When you change a model's SQL, `ol-dbt diff` compares two relations row-by-row and +column-by-column so you can confirm exactly what changed — instead of eyeballing +`select *` output. + +To isolate your code change from any upstream data drift, snapshot the model's +current build as a frozen baseline *before* making your change, then diff that +baseline against the rebuild: + +```bash +# 1. Freeze the current (pre-change) build as a baseline table +ol-dbt local snapshot my_model --as my_model_baseline + +# 2. Make your code change, then rebuild +ol-dbt run --select my_model --target dev_local + +# 3. Diff the frozen baseline against the rebuild +ol-dbt diff --target dev_local \ + --old my_model_baseline --old-raw \ + --new my_model \ + --primary-key my_model_pk +``` + +Because both sides are built from the same underlying data (the baseline is frozen, +the rebuild only reflects your code change), any reported mismatch can only come +from your change — never from production data changing in the background. + +```bash +# Per-column mismatch rates require --primary-key (repeatable for a composite key) +ol-dbt diff --old m_old --new m_new --primary-key id + +# Exclude a known non-deterministic column (e.g. a load timestamp) +ol-dbt diff --old m_old --new m_new -k id --exclude-columns _loaded_at + +# Build both sides first, then compare +ol-dbt diff --old m_old --new m_new --auto-build + +# Compare two already-materialized copies of the same model across schemas +# (e.g. your personal dev schema vs. production) on one Trino target +ol-dbt diff --target dev_production \ + --old my_model --old-raw --old-schema ol_warehouse_production_reporting \ + --new my_model --new-raw --new-schema ol_warehouse_production_rlougee_reporting \ + --primary-key id + +# Emit JSON (e.g. for CI); exits non-zero on any divergence +ol-dbt diff --old m_old --new m_new -k id --format json +``` + +`--old-raw`/`--new-raw` treat that side as a literal existing table rather than a +dbt model — required for a snapshot table (`ol-dbt local snapshot` output) or any +other already-materialized relation dbt doesn't know about via `ref()`. + ### Running dbt Directly You can still invoke dbt commands directly from `src/ol_dbt/`: diff --git a/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py b/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py index 3b52c8973..48948acdf 100644 --- a/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py +++ b/src/ol_dbt_cli/ol_dbt_cli/commands/diff.py @@ -21,11 +21,6 @@ literal ``api.Relation.create(...)`` instead — for relations dbt doesn't know about: an already-materialized table sitting in a different schema/database than the current --target. -* ``--old-glue``/``--new-glue`` are a variant of ``--old-raw``/``--new-raw`` - specifically for the ``glue__{glue_database}__{table}`` DuckDB views created by - ``ol-dbt local register`` — live views onto the real production Iceberg table, - not a copy. Pass just the Glue database name; ``--old``/``--new`` stay the bare - table name on both sides instead of the full ``glue__...`` identifier. Driveable both by hand and by CI (Phase 2 auto-diff vs a base ref). See docs/specs/DBT_WAREHOUSE_CI_QA_SPEC.md §3. @@ -123,12 +118,12 @@ class DiffResult: column_mismatches: list[ColumnMismatch] sample_mismatches: list[dict[str, Any]] verdict: Verdict - # Text-output-only labels annotating each side's source (e.g. "name (glue: - # db)") when --old/new-raw or --old/new-glue make `old`/`new` ambiguous — - # identical to `old`/`new` for a plain ref()-vs-ref() diff. Kept separate - # from `old`/`new` so JSON output stays a stable, bare identifier. Uses - # parens, not brackets — rich.console.print() parses `[...]` as markup and - # silently swallows anything that isn't a recognized style tag. + # Text-output-only labels annotating each side's source (e.g. "name (raw)") + # when --old/new-raw make `old`/`new` ambiguous — identical to `old`/`new` + # for a plain ref()-vs-ref() diff. Kept separate from `old`/`new` so JSON + # output stays a stable, bare identifier. Uses parens, not brackets — + # rich.console.print() parses `[...]` as markup and silently swallows + # anything that isn't a recognized style tag. old_label: str new_label: str notes: list[str] = field(default_factory=list) @@ -171,20 +166,19 @@ def _resolve_raw_columns( *, database: str | None = None, schema: str | None = None, - glue_database: str | None = None, ) -> tuple[set[str], str | None]: """Resolve a raw (non-``ref()``) relation's columns by sampling one row. - Raw relations (Glue-registered views, an already-materialized table in - another schema) have no manifest/YAML metadata to resolve columns from - statically, so this samples a live row and reads its keys instead. + Raw relations (an already-materialized table in another schema) have no + manifest/YAML metadata to resolve columns from statically, so this samples + a live row and reads its keys instead. Returns ``(columns, error)``. *error* carries the dbt failure message when the sampling query itself fails (e.g. the relation doesn't exist) — the caller must surface it rather than let an empty column set masquerade as "unparsed / not in manifest", which is misleading for a raw relation. """ - expr = _relation_jinja(name, raw=True, database=database, schema=schema, glue_database=glue_database) + expr = _relation_jinja(name, raw=True, database=database, schema=schema) # Rendered into a Jinja template compiled by dbt (not executed as a raw query # here), so the S608 heuristic is a false positive — same as _compare_column_sql. # No literal LIMIT here: `dbt show` already wraps the compiled query in its own @@ -198,31 +192,6 @@ def _resolve_raw_columns( return ({c.lower() for c in rows[0]} if rows else set()), None -def _materialize_glue_relation(name: str, glue_database: str, dbt_dir: Path, target: str) -> str: - """Copy a Glue-registered ``iceberg_scan()`` view into a real local DuckDB table. - - DuckDB's Iceberg extension can run a plain ``SELECT``/``COUNT(*)`` against a - live ``glue__...`` view (that's why column resolution works) but not the - ``UNION ALL``/``EXCEPT`` set operations audit_helper's compare macros - generate — that fails with ``Not implemented Error: IcebergScan - serialization not implemented``. Copying the view into a real table first - sidesteps the limitation entirely. - - Uses ``create or replace table`` with a name deterministic in - (*glue_database*, *name*), so repeated diffs reuse/refresh the same scratch - table instead of accumulating new ones. Returns the scratch table's bare - identifier (materialized in the target's own database/schema). - """ - scratch_name = f"_ol_dbt_diff_scratch__{glue_database}__{name}" - src_expr = _relation_jinja(name, glue_database=glue_database) - dst_expr = _relation_jinja(scratch_name, raw=True) - ctas_sql = f"create or replace table {{{{ {dst_expr} }}}} as select * from {{{{ {src_expr} }}}}" # noqa: S608 - # limit=-1: dbt show's own --limit wrapping would otherwise truncate the - # CTAS to N rows -- the whole point here is a full, unwrapped copy. - _run_dbt_show(ctas_sql, dbt_dir, target, limit=-1) - return scratch_name - - def reconcile_columns( old_cols: set[str], new_cols: set[str], @@ -359,7 +328,6 @@ def _relation_jinja( raw: bool = False, database: str | None = None, schema: str | None = None, - glue_database: str | None = None, ) -> str: """Render a relation reference for inline Jinja SQL. @@ -368,16 +336,7 @@ def _relation_jinja( instead, addressed by *database*/*schema* (defaulting to the current ``--target``'s own database/schema) — for relations dbt doesn't know about, e.g. an already-materialized table elsewhere. - - *glue_database*, when given, forces raw mode and rewrites the identifier to - the ``glue__{glue_database}__{name}`` naming convention that ``ol-dbt local - register`` uses for its DuckDB views (see ``local_dev.py``) — so callers can - pass the same bare table *name* for both a Glue-registered view and a real - dbt model instead of spelling out the full ``glue__...`` identifier. """ - if glue_database: - raw = True - name = f"glue__{glue_database}__{name}" if not raw: return f"ref('{name}')" db = f"'{database}'" if database else "target.database" @@ -398,8 +357,6 @@ def _compare_relations_sql( new_schema: str | None = None, old_database: str | None = None, new_database: str | None = None, - old_glue: str | None = None, - new_glue: str | None = None, ) -> str: """Build inline SQL calling ``audit_helper.compare_relations``. @@ -412,8 +369,8 @@ def _compare_relations_sql( pk_repr = f"'{primary_key[0]}'" if len(primary_key) == 1 else _jinja_list(primary_key) pk_arg = f", primary_key={pk_repr}" exclude_arg = f", exclude_columns={_jinja_list(exclude_columns)}" if exclude_columns else "" - a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema, glue_database=old_glue) - b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema, glue_database=new_glue) + a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema) + b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema) # `}}` in an f-string is an escape for a single literal `}` (same as `{{` for # `{`) — closing the Jinja `{{ ... }}` block from an f-string needs `}}}}`. return ( @@ -435,13 +392,11 @@ def _compare_column_sql( new_schema: str | None = None, old_database: str | None = None, new_database: str | None = None, - old_glue: str | None = None, - new_glue: str | None = None, ) -> str: """Build inline SQL calling ``audit_helper.compare_column_values`` for one column.""" pk = f"'{primary_key[0]}'" if len(primary_key) == 1 else _jinja_list(primary_key) - a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema, glue_database=old_glue) - b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema, glue_database=new_glue) + a_expr = _relation_jinja(old, raw=old_raw, database=old_database, schema=old_schema) + b_expr = _relation_jinja(new, raw=new_raw, database=new_database, schema=new_schema) # Only the primary key(s) + the one column being compared are needed here -- # `compare_column_values` does a full outer join on a_query/b_query, and # selecting every column (as this used to) forces that join to carry the @@ -644,8 +599,7 @@ def diff( Parameter( name=["--new"], help="Candidate model name (the 'after' relation). Defaults to --old's value if omitted — " - "the common case is comparing the same table name via --old-raw/--old-glue against its " - "ref()-resolved build.", + "the common case is comparing the same table name via --old-raw against its ref()-resolved build.", ), ] = None, dbt_dir_path: Annotated[ @@ -705,9 +659,9 @@ def diff( Parameter( name=["--old-raw"], help="Treat --old as a literal existing relation, not a dbt model — bypasses ref()/manifest " - "lookup. Use for a Glue-registered view (`ol-dbt local register`, a live view onto the real " - "production Iceberg table) or an already-materialized table dbt doesn't know about. Defaults " - "to the current --target's own database/schema; override with --old-database/--old-schema.", + "lookup. Use for an already-materialized table dbt doesn't know about (e.g. a snapshot made " + "with `ol-dbt local snapshot`). Defaults to the current --target's own database/schema; " + "override with --old-database/--old-schema.", ), ] = False, new_raw: Annotated[ @@ -742,30 +696,6 @@ def diff( help="Database/catalog override for --new. Requires --new-raw (default: the current --target's database).", ), ] = None, - old_glue: Annotated[ - str | None, - Parameter( - name=["--old-glue"], - help="Treat --old as a Glue-registered DuckDB view (`ol-dbt local register`) named " - "glue__{OLD_GLUE}__{old} — pass just the Glue database name and keep --old as the bare " - "table name. A live view onto the real production Iceberg table, not a copy. Implies " - "--old-raw; mutually exclusive with it.", - ), - ] = None, - new_glue: Annotated[ - str | None, - Parameter(name=["--new-glue"], help="Same as --old-glue, but for --new."), - ] = None, - refresh_glue: Annotated[ - bool, - Parameter( - name=["--refresh-glue"], - help="Run 'ol-dbt local register --all-layers' before comparing. Glue-registered views go " - "stale (HTTP 404 on their pinned S3 metadata) once production rebuilds the underlying table " - "-- this refreshes them first. Covers not just --old-glue/--new-glue but any upstream " - "Glue-registered dependency --auto-build's dbt build might hit via override_ref.sql.", - ), - ] = False, ) -> None: r"""Diff two dbt model relations (same-engine row/column comparison). @@ -784,17 +714,6 @@ def diff( Build both sides first, then compare: ol-dbt diff --old m_old --new m_new --auto-build - Compare a migrated model's local build against the real, already-materialized - production table via its Glue-registered view. --new defaults to --old's value, - so only one bare table name is needed. --refresh-glue re-registers Glue views - first (`ol-dbt local register --all-layers`) instead of requiring it to have - been run separately -- worthwhile whenever the production table might have - rebuilt (and its old __dbt_tmp snapshot been cleaned up) since you last - registered it, which surfaces as an HTTP 404 on stale S3 metadata: - ol-dbt diff \\ - --old enrollment_detail_report --old-glue ol_warehouse_production_reporting \\ - --primary-key courserunenrollment_id --refresh-glue - Compare two already-materialized copies of the same model across schemas (e.g. your personal dev schema vs. real production) on one Trino target: ol-dbt diff --target dev_production \\ @@ -802,29 +721,24 @@ def diff( --new enrollment_detail_report --new-raw --new-schema ol_warehouse_production_rlougee_reporting \\ --primary-key courserunenrollment_id + Compare a frozen baseline (`ol-dbt local snapshot`) against a rebuild after a + code change, to isolate the change from any upstream data drift: + ol-dbt local snapshot enrollment_detail_report --as enrollment_detail_report_baseline + # ... edit the model, then rebuild it ... + ol-dbt diff --old enrollment_detail_report_baseline --old-raw \\ + --new enrollment_detail_report --primary-key user_pk + """ new = new or old primary_key_list = list(primary_key) exclude_list = list(exclude_columns) notes: list[str] = [] - if old_raw and old_glue: - err_console.print("[red]Error:[/] --old-raw and --old-glue are mutually exclusive.") + if (old_schema or old_database) and not old_raw: + err_console.print("[red]Error:[/] --old-schema/--old-database require --old-raw.") sys.exit(1) - if new_raw and new_glue: - err_console.print("[red]Error:[/] --new-raw and --new-glue are mutually exclusive.") - sys.exit(1) - - # --old-glue/--new-glue are a --*-raw variant (see _relation_jinja), so schema/ - # database overrides are valid alongside either. - old_is_raw = old_raw or bool(old_glue) - new_is_raw = new_raw or bool(new_glue) - - if (old_schema or old_database) and not old_is_raw: - err_console.print("[red]Error:[/] --old-schema/--old-database require --old-raw or --old-glue.") - sys.exit(1) - if (new_schema or new_database) and not new_is_raw: - err_console.print("[red]Error:[/] --new-schema/--new-database require --new-raw or --new-glue.") + if (new_schema or new_database) and not new_raw: + err_console.print("[red]Error:[/] --new-schema/--new-database require --new-raw.") sys.exit(1) # Validate every value interpolated into inline Jinja SQL before use. @@ -836,40 +750,27 @@ def diff( "schema/database override", [v for v in (old_schema, new_schema, old_database, new_database) if v], ) - _validate_identifiers("glue database", [v for v in (old_glue, new_glue) if v]) except InvalidIdentifierError as exc: err_console.print(f"[red]Error:[/] {exc}") sys.exit(1) - if old_glue: - notes.append( - f"--old-glue: '{old}' resolved as Glue-registered view " - f"'glue__{old_glue}__{old}' (database={old_database or 'target.database'}, " - f"schema={old_schema or 'target.schema'})." - ) - elif old_raw: + if old_raw: notes.append( f"--old-raw: '{old}' resolved as a literal relation " f"(database={old_database or 'target.database'}, schema={old_schema or 'target.schema'})." ) - if new_glue: - notes.append( - f"--new-glue: '{new}' resolved as Glue-registered view " - f"'glue__{new_glue}__{new}' (database={new_database or 'target.database'}, " - f"schema={new_schema or 'target.schema'})." - ) - elif new_raw: + if new_raw: notes.append( f"--new-raw: '{new}' resolved as a literal relation " f"(database={new_database or 'target.database'}, schema={new_schema or 'target.schema'})." ) - # Text-output labels disambiguating which side is which when --old/new-raw or - # --old/new-glue make the bare `old`/`new` names identical or otherwise - # ambiguous (e.g. both "enrollment_detail_report"). Unannotated for a plain - # ref()-vs-ref() diff, where `old`/`new` are already unambiguous. - old_label = f"{old} (glue:{old_glue})" if old_glue else (f"{old} (raw)" if old_raw else old) - new_label = f"{new} (glue:{new_glue})" if new_glue else (f"{new} (raw)" if new_raw else new) + # Text-output labels disambiguating which side is which when --old/new-raw + # make the bare `old`/`new` names identical or otherwise ambiguous (e.g. + # both "enrollment_detail_report"). Unannotated for a plain ref()-vs-ref() + # diff, where `old`/`new` are already unambiguous. + old_label = f"{old} (raw)" if old_raw else old + new_label = f"{new} (raw)" if new_raw else new # Resolve dbt project directory (shared preamble with impact/validate). if dbt_dir_path: @@ -893,53 +794,6 @@ def diff( err_console.print(f" Run: cd {dbt_dir} && dbt deps") sys.exit(1) - # Refresh Glue-registered views BEFORE anything below reads one -- both the - # materialization just below and --auto-build's `dbt build` (whose compiled - # SQL can hit other Glue-registered views transitively, via override_ref.sql) - # can fail with a stale-metadata 404 once production rebuilds the underlying - # table out from under a previously-registered view. - if refresh_glue: - if output_format == "text": - console.print("[dim]Running: ol-dbt local register --all-layers ...[/]") - try: - subprocess.run( # noqa: S603, S607 - ["ol-dbt", "local", "register", "--all-layers"], - capture_output=True, - text=True, - check=True, - ) - except subprocess.CalledProcessError as exc: - detail = (exc.stderr or exc.stdout or str(exc)).strip() - err_console.print(f"[red]Error:[/] ol-dbt local register --all-layers failed: {detail[-500:]}") - sys.exit(1) - except FileNotFoundError: - err_console.print("[red]Error:[/] 'ol-dbt' command not found; ensure it is on PATH.") - sys.exit(1) - - # --old-glue/--new-glue relations are backed by iceberg_scan() views, which - # DuckDB's Iceberg extension can't serialize into audit_helper's UNION - # ALL/EXCEPT comparison SQL (see _materialize_glue_relation). Swap them for a - # materialized copy before any column resolution or comparison runs. These - # `*_query` variables are the ones actually used to build SQL below; - # `old`/`new`/`old_label`/`new_label` are left untouched so notes/output keep - # showing the original Glue identifier the user asked to compare. - old_query, old_query_glue, old_query_database, old_query_schema = old, old_glue, old_database, old_schema - new_query, new_query_glue, new_query_database, new_query_schema = new, new_glue, new_database, new_schema - if old_glue: - try: - old_query = _materialize_glue_relation(old, old_glue, dbt_dir, target) - except RuntimeError as exc: - err_console.print(f"[red]Error:[/] failed to materialize --old-glue relation: {exc}") - sys.exit(1) - old_query_glue, old_query_database, old_query_schema = None, None, None - if new_glue: - try: - new_query = _materialize_glue_relation(new, new_glue, dbt_dir, target) - except RuntimeError as exc: - err_console.print(f"[red]Error:[/] failed to materialize --new-glue relation: {exc}") - sys.exit(1) - new_query_glue, new_query_database, new_query_schema = None, None, None - models_dir = dbt_dir / "models" # Build the metadata sources used for column reconciliation. @@ -963,26 +817,12 @@ def diff( # --- Column reconciliation (before any comparison) --- old_raw_error: str | None = None new_raw_error: str | None = None - if old_is_raw: - old_cols, old_raw_error = _resolve_raw_columns( - old_query, - dbt_dir, - target, - database=old_query_database, - schema=old_query_schema, - glue_database=old_query_glue, - ) + if old_raw: + old_cols, old_raw_error = _resolve_raw_columns(old, dbt_dir, target, database=old_database, schema=old_schema) else: old_cols = _resolve_columns(old, sql_models_by_name, manifest, yaml_registry) - if new_is_raw: - new_cols, new_raw_error = _resolve_raw_columns( - new_query, - dbt_dir, - target, - database=new_query_database, - schema=new_query_schema, - glue_database=new_query_glue, - ) + if new_raw: + new_cols, new_raw_error = _resolve_raw_columns(new, dbt_dir, target, database=new_database, schema=new_schema) else: new_cols = _resolve_columns(new, sql_models_by_name, manifest, yaml_registry) @@ -1020,10 +860,10 @@ def diff( ) # Optionally build both relations first. Raw relations aren't dbt models — - # they're assumed to already exist (a Glue view, an already-materialized - # table) — so they're excluded from --select rather than passed to dbt build. + # they're assumed to already exist (an already-materialized table) — so + # they're excluded from --select rather than passed to dbt build. if auto_build: - build_targets = [name for name, raw in ((old, old_is_raw), (new, new_is_raw)) if not raw] + build_targets = [name for name, raw in ((old, old_raw), (new, new_raw)) if not raw] if not build_targets: notes.append("--auto-build skipped: both --old and --new are raw relations (nothing to build).") else: @@ -1074,19 +914,17 @@ def diff( try: summary_rows = _run_dbt_show( _compare_relations_sql( - old_query, - new_query, + old, + new, primary_key_list, exclude_list, summarize=True, - old_raw=old_is_raw, - new_raw=new_is_raw, - old_schema=old_query_schema, - new_schema=new_query_schema, - old_database=old_query_database, - new_database=new_query_database, - old_glue=old_query_glue, - new_glue=new_query_glue, + old_raw=old_raw, + new_raw=new_raw, + old_schema=old_schema, + new_schema=new_schema, + old_database=old_database, + new_database=new_database, ), dbt_dir, target, @@ -1097,19 +935,17 @@ def diff( if mismatched_rows: sample_mismatches = _run_dbt_show( _compare_relations_sql( - old_query, - new_query, + old, + new, primary_key_list, exclude_list, summarize=False, - old_raw=old_is_raw, - new_raw=new_is_raw, - old_schema=old_query_schema, - new_schema=new_query_schema, - old_database=old_query_database, - new_database=new_query_database, - old_glue=old_query_glue, - new_glue=new_query_glue, + old_raw=old_raw, + new_raw=new_raw, + old_schema=old_schema, + new_schema=new_schema, + old_database=old_database, + new_database=new_database, ), dbt_dir, target, @@ -1124,20 +960,18 @@ def diff( cols = cols[:_MAX_PER_COLUMN_COMPARISONS] for col in cols: mismatch = _compare_single_column( - old_query, - new_query, + old, + new, primary_key_list, col, dbt_dir, target, - old_raw=old_is_raw, - new_raw=new_is_raw, - old_schema=old_query_schema, - new_schema=new_query_schema, - old_database=old_query_database, - new_database=new_query_database, - old_glue=old_query_glue, - new_glue=new_query_glue, + old_raw=old_raw, + new_raw=new_raw, + old_schema=old_schema, + new_schema=new_schema, + old_database=old_database, + new_database=new_database, ) if mismatch is not None and mismatch.mismatched_rows > 0: column_mismatches.append(mismatch) @@ -1197,8 +1031,6 @@ def _compare_single_column( new_schema: str | None = None, old_database: str | None = None, new_database: str | None = None, - old_glue: str | None = None, - new_glue: str | None = None, ) -> ColumnMismatch | None: """Run a per-column value comparison, returning its mismatch rate. @@ -1217,8 +1049,6 @@ def _compare_single_column( new_schema=new_schema, old_database=old_database, new_database=new_database, - old_glue=old_glue, - new_glue=new_glue, ), dbt_dir, target, diff --git a/src/ol_dbt_cli/tests/test_diff.py b/src/ol_dbt_cli/tests/test_diff.py index 4259f4253..c58b7d3e2 100644 --- a/src/ol_dbt_cli/tests/test_diff.py +++ b/src/ol_dbt_cli/tests/test_diff.py @@ -251,26 +251,6 @@ def test_compare_column_sql_narrows_to_primary_key_and_column(self) -> None: assert "select id, some_col from" in sql assert "select *" not in sql - def test_glue_database_rewrites_identifier_and_forces_raw(self) -> None: - expr = _relation_jinja("enrollment_detail_report", glue_database="ol_warehouse_production_reporting") - assert expr == ( - "api.Relation.create(database=target.database, schema=target.schema, " - "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report')" - ) - - def test_compare_relations_sql_with_old_glue(self) -> None: - sql = _compare_relations_sql( - "enrollment_detail_report", - "enrollment_detail_report", - ["id"], - [], - summarize=True, - old_glue="ol_warehouse_production_reporting", - ) - assert "ref('enrollment_detail_report')" in sql # --new side unaffected - assert "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in sql - assert "ref('glue__" not in sql - class TestResolveRawColumns: def test_returns_lowercased_keys_from_sampled_row(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: @@ -589,161 +569,19 @@ def boom(*a: Any, **k: Any) -> Any: dbt_dir_path=str(dbt_dir), ) - def test_refresh_glue_registers_all_layers_before_comparing( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch - ) -> None: - # --refresh-glue must run before any Glue-registered view is read -- - # both the --old-glue/--new-glue materialization and (for --auto-build) - # dbt build's own upstream deps can 404 on stale S3 metadata otherwise. - dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") - monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: [{"in_a": True, "in_b": True, "count": 1}]) - calls: list[list[str]] = [] - - def fake_run(cmd: list[str], **kwargs: Any) -> Any: - calls.append(cmd) - import subprocess - - return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") - - monkeypatch.setattr(diff_mod.subprocess, "run", fake_run) - diff(old="m_old", new="m_new", refresh_glue=True, dbt_dir_path=str(dbt_dir)) - assert calls[0] == ["ol-dbt", "local", "register", "--all-layers"] - - -class TestDiffCommandGlue: - def test_old_glue_and_old_raw_together_exits_1(self, tmp_path: Path) -> None: - dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") - with pytest.raises(SystemExit) as exc: - diff( - old="m_old", - new="m_new", - old_raw=True, - old_glue="ol_warehouse_production_reporting", - dbt_dir_path=str(dbt_dir), - ) - assert exc.value.code == 1 - - def test_invalid_glue_database_rejected(self, tmp_path: Path) -> None: - dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") - with pytest.raises(SystemExit) as exc: - diff(old="m_old", new="m_new", old_glue="a; drop table x", dbt_dir_path=str(dbt_dir)) - assert exc.value.code == 1 - - def test_old_glue_keeps_bare_name_and_rewrites_identifier( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] - ) -> None: - # --old and --new both stay "enrollment_detail_report" as the underlying - # `old`/`new` identifiers — only the SQL identifier and the printed label - # should carry the glue__/(glue:...) annotation. - dbt_dir = _make_project( - tmp_path, - "select 1 as id, 'x' as name", - "select 1 as id, 'x' as name", - new_name="enrollment_detail_report", - ) - seen_inline: list[str] = [] - - def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: - seen_inline.append(inline_sql) - if "audit_helper" not in inline_sql: - return [{"id": 1, "name": "x"}] # raw column sample - return [{"in_a": True, "in_b": True, "count": 10}] - - monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) - diff( - old="enrollment_detail_report", - new="enrollment_detail_report", - old_glue="ol_warehouse_production_reporting", - dbt_dir_path=str(dbt_dir), - ) - out = " ".join(capsys.readouterr().out.split()) # rich line-wraps the long header - assert "enrollment_detail_report (glue:ol_warehouse_production_reporting) → enrollment_detail_report" in out - assert not any("ref('glue__" in sql for sql in seen_inline) - assert any( - "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in sql - for sql in seen_inline - ) - assert any("ref('enrollment_detail_report')" in sql for sql in seen_inline) - - def test_old_glue_materializes_scratch_table_before_comparing( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch - ) -> None: - # DuckDB's Iceberg extension can't run audit_helper's UNION ALL/EXCEPT SQL - # against a live glue__ view (IcebergScan serialization not implemented), - # so the glue side must be copied into a real scratch table first, and - # every downstream comparison query must reference that scratch table -- - # never the live glue__ identifier -- or the same crash resurfaces. - dbt_dir = _make_project( - tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name", new_name="enrollment_detail_report" - ) - seen_inline: list[str] = [] - - def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: - seen_inline.append(inline_sql) - if "audit_helper" not in inline_sql: - return [{"id": 1, "name": "x"}] - return [{"in_a": True, "in_b": True, "count": 10}] - - monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) - diff( - old="enrollment_detail_report", - new="enrollment_detail_report", - old_glue="ol_warehouse_production_reporting", - dbt_dir_path=str(dbt_dir), - ) - ctas_calls = [sql for sql in seen_inline if "create or replace table" in sql] - assert len(ctas_calls) == 1 - assert ( - "identifier='_ol_dbt_diff_scratch__ol_warehouse_production_reporting__enrollment_detail_report'" - in (ctas_calls[0]) - ) - assert "identifier='glue__ol_warehouse_production_reporting__enrollment_detail_report'" in ctas_calls[0] - - compare_calls = [sql for sql in seen_inline if "audit_helper" in sql] - assert compare_calls # summarize + (no mismatches -> no sample) at least one - assert not any("glue__" in sql for sql in compare_calls) - assert all( - "identifier='_ol_dbt_diff_scratch__ol_warehouse_production_reporting__enrollment_detail_report'" in sql - for sql in compare_calls - ) - - def test_old_glue_sampling_failure_surfaces_real_dbt_error( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] - ) -> None: - # If the Glue view doesn't exist (wrong glue database, not registered yet, - # ...), materializing it (which now runs before column resolution, to work - # around DuckDB's IcebergScan/UNION-ALL limitation) fails first — the real - # dbt error must be surfaced, not the generic "unparsed / not in manifest" - # note written for the static-parse path. - dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") - - def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: - msg = "dbt show failed: Relation glue__bad_db__m_old does not exist" - raise RuntimeError(msg) - - monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) - with pytest.raises(SystemExit) as exc: - diff(old="m_old", new="m_new", old_glue="bad_db", dbt_dir_path=str(dbt_dir)) - assert exc.value.code == 1 - err = " ".join(capsys.readouterr().err.split()) # rich line-wraps long notes - assert "failed to materialize --old-glue relation" in err - assert "Relation glue__bad_db__m_old does not exist" in err - assert "not in manifest" not in err - class TestDiffCommandLabels: def test_plain_diff_labels_are_unannotated( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - # A plain ref()-vs-ref() diff (no --*-raw/--*-glue) is already unambiguous - # — the header must not grow a "(raw)"/"(glue:...)" annotation. + # A plain ref()-vs-ref() diff (no --*-raw) is already unambiguous — the + # header must not grow a "(raw)" annotation. dbt_dir = _make_project(tmp_path, "select 1 as id", "select 1 as id") monkeypatch.setattr(diff_mod, "_run_dbt_show", lambda *a, **k: [{"in_a": True, "in_b": True, "count": 1}]) diff(old="m_old", new="m_new", dbt_dir_path=str(dbt_dir)) out = capsys.readouterr().out assert "m_old → m_new" in out assert "(raw)" not in out - assert "(glue:" not in out def test_old_raw_label_annotated_with_raw( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] @@ -759,18 +597,3 @@ def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: diff(old="glue__x__y", new="m_new", old_raw=True, dbt_dir_path=str(dbt_dir)) out = capsys.readouterr().out assert "glue__x__y (raw) → m_new" in out - - def test_old_glue_label_annotated_with_glue_database( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] - ) -> None: - dbt_dir = _make_project(tmp_path, "select 1 as id, 'x' as name", "select 1 as id, 'x' as name") - - def fake_show(inline_sql: str, *a: Any, **k: Any) -> list[dict[str, Any]]: - if "audit_helper" not in inline_sql: - return [{"id": 1, "name": "x"}] - return [{"in_a": True, "in_b": True, "count": 1}] - - monkeypatch.setattr(diff_mod, "_run_dbt_show", fake_show) - diff(old="m_old", new="m_new", old_glue="ol_warehouse_production_reporting", dbt_dir_path=str(dbt_dir)) - out = capsys.readouterr().out - assert "m_old (glue:ol_warehouse_production_reporting) → m_new" in out From a2763a4c55decc9aa6d8d9bbb812414901a3071a Mon Sep 17 00:00:00 2001 From: rlougee Date: Wed, 22 Jul 2026 14:44:56 -0400 Subject: [PATCH 3/3] feat(local_dev): add dbt project validation and error handling --- src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py | 10 ++++++++++ src/ol_dbt_cli/tests/test_local_dev.py | 1 + 2 files changed, 11 insertions(+) diff --git a/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py b/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py index 84c554c8c..ea3e6da53 100644 --- a/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py +++ b/src/ol_dbt_cli/ol_dbt_cli/commands/local_dev.py @@ -720,6 +720,16 @@ def snapshot( print(f"Error: {exc}") sys.exit(1) + # _find_dbt_dir returns an explicit --dbt-dir as-is, without checking it's a + # real dbt project -- a typo'd path would otherwise only surface later as a + # confusing dbt-level error instead of a clear, early failure here. + if not (dbt_dir / "dbt_project.yml").exists(): + print( + f"Error: dbt project not found at {dbt_dir}. " + "Pass --dbt-dir pointing at a directory containing dbt_project.yml." + ) + sys.exit(1) + # Rendered into a Jinja template compiled by dbt (not executed as a raw query # here), so the S608 heuristic is a false positive -- model/as_name were # already validated as plain identifiers above. diff --git a/src/ol_dbt_cli/tests/test_local_dev.py b/src/ol_dbt_cli/tests/test_local_dev.py index 36d1a218b..7696fc8b6 100644 --- a/src/ol_dbt_cli/tests/test_local_dev.py +++ b/src/ol_dbt_cli/tests/test_local_dev.py @@ -19,6 +19,7 @@ class TestSnapshot: def test_runs_ctas_with_validated_identifiers(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: import subprocess + (tmp_path / "dbt_project.yml").write_text("name: test\nprofile: test\n") calls: list[list[str]] = [] def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: