Skip to content

fix: generate a valid duckdb gateway config for DuckLake destinations (#5914) - #6118

Open
AnnasMazhar wants to merge 2 commits into
SQLMesh:mainfrom
AnnasMazhar:fix/5914-ducklake-catalog-error
Open

AnnasMazhar wants to merge 2 commits into
SQLMesh:mainfrom
AnnasMazhar:fix/5914-ducklake-catalog-error

Conversation

@AnnasMazhar

Copy link
Copy Markdown

Problem

A ducklake destination in config.yaml produced an invalid gateway config. The failure surfaced from
connection.py as ConfigError: Unknown connection type 'ducklake'. — a message identical to the one
raised for a genuinely unknown type, so the real diagnostic was lost.

DuckLake is a DuckDB gateway with the lake attached as a catalog, so the config needs that block built
explicitly rather than falling through the generic credentials path.

Fix

sqlmesh/integrations/dlt.py

  • Build the duckdb gateway block for a ducklake destination: the catalog and the ATTACH SQL.
  • Quote every interpolated scalar through a small helper. Previously catalog.database,
    storage_url, ducklake_name and metadata_schema were interpolated raw, so a path containing a
    leading quote or a newline produced invalid YAML and the generated project would not load.
  • Cut unsupported catalog drivers to a clear ClickException instead of letting the generic error
    propagate.
  • Replace client_config: t.Any with a TYPE_CHECKING forward reference, so the module still imports
    when the optional ducklake dependency is absent.
  • Document a known limitation at the fall-through: any other unrecognised db_type
    (weaviate, pandas, qdrant, typos) still raises
    ConfigError: Unknown connection type '<type>'. at that boundary. That behaviour is unchanged by this
    pull request. This issue reports the ducklake destination specifically
    ("When using dlt with a ducklake destination ..."), so the narrower fix is deliberate. I am happy to
    widen it to a generic guard at that boundary if you would prefer that here.

Tests

tests/cli/test_cli.py

8 tests added, 1 strengthened:

  • test_dlt_ducklake_pipeline — parses the emitted YAML and asserts on structure, not substrings
  • test_dlt_ducklake_explicit_metadata_schema, ..._override_data_path, ..._custom_name
  • test_dlt_ducklake_pathological_paths_round_trip — leading quote, newline, : #, leading space
  • test_dlt_ducklake_yaml_inline_helper, ..._block_coexists_with_second_catalog
  • test_dlt_ducklake_unsupported_catalog — asserts the specific driver in the message and spies the
    branch, so it cannot pass for the wrong reason
pytest tests/cli/test_cli.py -k "ducklake or test_dlt_pipeline"          -> 12 passed
pytest tests/core/test_connection_config.py -k "ducklake or duckdb_attach" -> 5 passed
ruff check -> All checks passed        ruff format --check -> 2 files already formatted
mypy sqlmesh/integrations/dlt.py -> 0 errors

Ordinary paths emit byte-identical YAML to the previous commit; the else branch (non-ducklake,
non-filesystem) is byte-identical to main. The new tests fail when the source fix is stashed and pass
with it.

Notes

A `ducklake` entry in config.yaml fell through the generic credentials path and
produced an invalid duckdb gateway config, which surfaced as a ConfigError from
connection.py with a message identical to an unrelated failure — so the real
diagnostic was lost. DuckLake connections are a duckdb gateway with the lake
attached as a catalog, so build that block explicitly.

Unsupported catalog drivers are cut to a clear ClickException rather than
propagating the generic error; postgres/mysql/MotherDuck catalogs are not yet
mapped and are tracked on the issue.

Tests: 3 new (duckdb catalog, sqlite catalog, unsupported driver) plus the
existing dlt pipeline tests. Verified to fail on unpatched source and pass on
this commit.

Signed-off-by: Syed Annas <annas.mazhar10@gmail.com>
Signed-off-by: Syed Annas <annas.mazhar10@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant