Skip to content

fix(dlt): compare dlt version semantically for destination client#5890

Merged
StuffbyYuki merged 1 commit into
SQLMesh:mainfrom
anxkhn:fix/dlt-version-parse
Jul 10, 2026
Merged

fix(dlt): compare dlt version semantically for destination client#5890
StuffbyYuki merged 1 commit into
SQLMesh:mainfrom
anxkhn:fix/dlt-version-parse

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The dlt destination-client selection in sqlmesh/integrations/dlt.py gated the
1.10.0 API change with a string comparison:

if dlt.__version__ >= "1.10.0":
    client = pipeline.destination_client()
else:
    client = pipeline._sql_job_client(schema)  # type: ignore

String comparison is lexicographic, not semantic, so it is wrong for any version
with a double-digit minor. For example "1.9.0" >= "1.10.0" evaluates to True
(the character '9' sorts after '1'), and likewise for "1.2.0" and
"1.5.10". The result is that every 1.x version takes the >= 1.10.0 branch,
and the pre-1.10 _sql_job_client fallback (added in #4223 for compatibility
with older dlt) is unreachable.

This is currently latent rather than crashing, because dlt versions below 1.10.0
happen to expose destination_client() as well. But the version guard is
silently incorrect and the intended fallback never runs.

The fix uses packaging.version.parse for a semantic comparison, mirroring the
idiom already used in sqlmesh/core/config/connection.py
(version.parse(...) < version.parse("...")). packaging is already a
top-level dependency, so no new dependency is introduced.

The client selection is extracted into a small _get_destination_client helper
so the version gate can be unit tested without a live dlt pipeline.

Test Plan

  • Added test_get_destination_client_version_gate in
    tests/integrations/test_dlt.py: a parametrized, pure-Python test that
    monkeypatches dlt.__version__ and asserts 1.9.0, 1.2.0, 1.5.10 select
    _sql_job_client while 1.10.0, 1.28.1, 2.0.0 select
    destination_client. It fails under the old string comparison for the pre-1.10
    cases and passes after the fix. No warehouse or live dlt pipeline required.
  • pytest tests/integrations/test_dlt.py -> 7 passed.
  • make style (ruff check, ruff format, mypy) clean.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

@StuffbyYuki StuffbyYuki left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

Given destination_client() exists on all realistically supported dlt versions and is what everyone already hits due to the string-compare bug, would you consider simplifying to always use destination_client() (or pinning dlt>=1.10) instead of maintaining a version gate?

Let me know what you think

@anxkhn anxkhn force-pushed the fix/dlt-version-parse branch from 71733d8 to 899bafd Compare July 9, 2026 17:05
@anxkhn

anxkhn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion. I simplified this to always use destination_client() and removed the version gate / private _sql_job_client fallback.

I also rebased on latest main and re-ran the targeted checks locally:

  • uv run pytest tests/integrations/test_dlt.py
  • uv run --extra dev ruff check sqlmesh/integrations/dlt.py tests/integrations/test_dlt.py
  • uv run --extra dev ruff format --check sqlmesh/integrations/dlt.py tests/integrations/test_dlt.py

The failing test-dbt-versions (1.3) CI job appears unrelated to this dlt change. It is failing in Snowflake / pyOpenSSL import setup.
Also update the PR description after pushing, since it still mentions the removed semantic-version helper/test.

@anxkhn anxkhn requested a review from StuffbyYuki July 9, 2026 17:06
@StuffbyYuki

Copy link
Copy Markdown
Collaborator

@anxkhn Can you rebase your branch? there was a fix that got merged that fixes the test-dbt-version ci test

The dlt destination-client selection gated the 1.10.0 API change with a
string comparison (`dlt.__version__ >= "1.10.0"`). String comparison is
lexicographic, so it wrongly evaluated "1.9.0" >= "1.10.0" (and "1.2.0",
"1.5.10", ...) as True, sending every 1.x version down the >=1.10.0
branch.

`destination_client()` is the public accessor on every realistically
supported dlt version, and the pre-1.10 `_sql_job_client` fallback was
already unreachable because of the same string-compare bug, so drop the
version gate and always use `destination_client()`.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@StuffbyYuki StuffbyYuki force-pushed the fix/dlt-version-parse branch from 899bafd to 57678e4 Compare July 10, 2026 06:05
@StuffbyYuki StuffbyYuki merged commit 9460918 into SQLMesh:main Jul 10, 2026
32 checks passed
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.

2 participants