You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(tests): stop Windows xdist workers from sharing DuckDB and sushi files - #6184
.github/workflows/pr.yaml (cicd-tests-windows)
+ Install DuckLake extension # once, before pytest
tests/core/engine_adapter/test_duckdb.py
ducklake_conn fixture (used by the 3 DuckLake tests, grouped with xdist_group)
- duck_conn.install_extension("ducklake")
duck_conn.load_extension("ducklake") # auto-installs when missing
tests/core/linter/test_helpers.py
- Context(paths=["examples/sushi"])+ Context(paths=copy_to_temp_path("examples/sushi"))
test_get_position_of_model_block
test_get_range_of_a_key_in_model_block_testing_on_sushi
Two Windows CI flakes, both from tests sharing files on disk. This unblocks #6176.
DuckLake. Once a process loads DuckLake, its DLL stays loaded until the process exits; closing the connection does not unload it. Any later install_extension("ducklake") in that process replaces the file on disk, which Windows refuses with Could not move file: Access is denied. Guarding the install did not help: in run 41899285609, load_extension in the second DuckLake test failed with Failed to create directory "C:\Users\runneradmin\.duckdb": Cannot create a file when that file already exists, even though the first test had just installed DuckLake there, and the install fallback then hit the loaded DLL. So the tests no longer install at all. LOAD auto-installs DuckLake when it is missing (local runs, Linux, macOS), and the Windows job installs it before pytest.
The transient ~/.duckdb error itself is not explained. Most likely another xdist worker is writing to the shared ~/.duckdb at the same moment. If it comes back, it now surfaces as that error instead of "Access is denied".
Sushi cache. The two linter tests wrote examples/sushi/.cache in the repo, and parallel workers collided on it. They now run on a temp copy.
Roughly 40 other tests still construct Context directly on examples/sushi and share that cache. This PR only isolates the two that failed. Worth a follow-up if more of them flake on Windows.
Test Plan
Before: Windows CI on PR 6176 (job 125057540476): test_drop_table_ducklake_no_cascade_mixed_case_alias failed (Access is denied) and test_get_range_of_a_key_in_model_block_testing_on_sushi failed (Permission denied on the sushi cache).
Earlier attempts on this branch all still failed on DuckLake: grouping alone (run 41408530674), explicit close() (run 41478055750), a duckdb_extensions().installed check (run 41636441149), and load-then-install (run 41899285609). The sushi cache failure did not return after the first commit. The installed-check code also passed once (run 41750858262), so one green run is not conclusive on its own.
After: Windows CI on 142d27b (job 126837113282): DuckLake pre-install step passed; fast tests 2814 passed, 5 skipped. The follow-up commit 6aad986 only reverts duck_conn in tests/conftest.py to main.
Local: macOS, the three DuckLake tests pass, and load_extension("ducklake") against an empty extension_directory auto-installs it. Linux and macOS can't reproduce the Windows failure.
Merge danger: two-way door, revert the commits. Blast radius is CI only; these are test and CI-workflow changes, and production engines are unchanged.
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
- tests/core/engine_adapter/test_duckdb.py: three tests that call
duck_conn.install_extension("ducklake") now share the same
xdist_group("ducklake_extension"). On Windows, parallel xdist workers
all downloading and renaming the extension binary at once triggers
'Could not move file: Access is denied'. Grouping them forces the
three tests onto a single worker, so extension installation is always
sequential.
- tests/core/linter/test_helpers.py: both sushi-context tests now use
the copy_to_temp_path fixture instead of pointing directly at
examples/sushi. The shared .cache/optimized_query directory is
excluded by the copytree ignore rules, so each test gets its own
writable cache root and Windows file-lock contention is eliminated.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Walkthrough
DuckLake tests now share extension setup, and the DuckDB connection fixture closes connections during teardown. Windows CI installs and loads DuckLake before fast unit tests. Two linter helper tests use temporary copies of the sushi examples.
Changes: DuckLake test setup and connection cleanup
Layer / File(s)
Summary
Set up DuckLake tests tests/core/engine_adapter/test_duckdb.py
A shared helper attempts to load DuckLake before installing it. Tests cover load success and install fallback, and DuckLake integration tests use the shared fixture.
Close connections and prepare Windows CI tests/conftest.py, .github/workflows/pr.yaml
The duck_conn fixture closes its connection during teardown. Windows CI installs and loads DuckLake before running fast unit tests.
Linter test paths
Layer / File(s)
Summary
Use temporary example copies tests/core/linter/test_helpers.py
Two helper tests pass a temporary copy of examples/sushi to Context.
SQLMesh/sqlmesh#6182: The issue reports Windows xdist workers racing during DuckLake installation. This PR changes the setup used by the same DuckLake tests.
Possibly related PRs
SQLMesh/sqlmesh#6183: Both PRs address concurrent DuckLake extension installation in the same tests.
Suggested reviewers:kingjaiteh
Merge Risk: Minimal · up to 2514d
No material code risk was identified. The reported DCO check failure still needs attention.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly identifies the main change: preventing Windows xdist workers from sharing DuckDB and sushi files.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description follows the repository template closely. It has the Description, Test Plan, and Checklist sections. The Description explains the DuckLake change (installs removed, LOAD auto-installs, …
Full details: Description check
Explanation
The description follows the repository template closely. It has the Description, Test Plan, and Checklist sections. The Description explains the DuckLake change (installs removed, LOAD auto-installs, Windows job pre-installs DuckLake) and the sushi cache change (temp copy for two linter tests), with the root causes and the Windows CI evidence. The Test Plan lists before and after CI runs, local macOS results, and the merge risk. The checklist marks style, existing tests, and DCO as done. The one unchecked item is 'I have added tests for my changes', which is a change-specific gap, since the PR only modifies existing tests. The description also notes that about 40 other tests share the sushi cache and are out of scope, which is useful context.
Full details: Docstring Coverage
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 1afdf145-67a4-4651-a6fc-ffdb77b95e4c
Commits
Reviewing files that changed from the base of the PR and between b7aa915 and 2514d48.
Files selected for processing: 3
.github/workflows/pr.yaml
tests/conftest.py
tests/core/engine_adapter/test_duckdb.py
Files skipped from review as they are similar to previous changes: 1
tests/conftest.py
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
… locks
On Windows, a loaded DuckDB extension (.dll) holds an OS file handle that
blocks install_extension() from atomically moving the newly-downloaded file
into place. The duck_conn fixture previously only returned the connection
(no explicit close), relying on CPython reference-counting to tear it down
between tests. With the adapter fixture holding the connection via a lambda
closure, and pytest-xdist running three ducklake tests sequentially on the
same worker, two stale connections can still hold the ducklake.dll open by
the time the third test calls install_extension(), producing:
_duckdb.IOException: IO Error: Could not move file: Access is denied.
Change the fixture to yield + conn.close() so the connection — and any DLLs
it has loaded — is fully released before the next test begins.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
Windows keeps ducklake.dll mapped after the connection closes, so a later
test's install_extension cannot move the file into place.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
A fixture loads DuckLake once per test without a second install call
while the DLL is still mapped, which on Windows causes 'Access is
denied' when install_extension tries to move ducklake.dll.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
The sushi copy_to_temp_path change looks good: on main those two tests write examples/sushi/.cache into the repo, and with this PR they don't. The DuckLake part still fails on Windows, though.
Blocking
The DuckLake fix doesn't hold on Windows. On the current head (b7aa915), cicd-tests-windows fails with:
ERROR at setup of test_drop_table_ducklake_no_cascade
> duck_conn.install_extension("ducklake")
E _duckdb.IOException: IO Error: Could not move file: Access is denied.
So the installed check in duckdb_extensions() said ducklake wasn't installed, and the install ran anyway, even with all three tests pinned to one worker via loadgroup. The earlier commits (grouping alone, then close()) also failed on Windows. A sturdier approach might be to try load_extension("ducklake") first and only install if that raises, or to pre-install ducklake once in a CI step before pytest. Either way, please show it green on Windows CI before we merge, since Linux and macOS can't reproduce this.
The description is out of date. It says Windows CI hasn't re-run this branch, but it has run three times and failed each time. Please update the evidence section once there's a passing run.
Non-blocking
The comment on duck_conn in tests/conftest.py says closing the connection releases loaded extension DLLs. DuckDB doesn't unload extensions when a connection closes, and the CI result fits that. The yield/close() change is fine to keep, but please reword or drop that explanation. The same goes for the ducklake_conn docstring if the approach changes.
About 40 other tests still use Context(paths=["examples/sushi"]) directly and share the same cache. It's fine to fix only the two that flaked here, but worth a note in the description or a follow-up issue.
The 3.9 failure (test_freeze_time_concurrent) looks unrelated to this PR.
duckdb_extensions().installed can report false on a new connection after
an earlier connection in the same process loaded the DLL, so the install
still ran and Windows denied the file move. Install only when load fails.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
Windows CI showed load_extension failing with a transient "Failed to
create directory ~/.duckdb" error, after which the install fallback hit
the already-loaded DLL. Never install from the fixture; LOAD auto-installs
when missing, and Windows CI installs DuckLake before pytest.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
Closing the connection does not unload extension DLLs, so the yield/close
change did not help the Windows DuckLake failure.
Signed-off-by: Nick Muoh <[email protected]>
Co-authored-by: Cursor <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two Windows CI flakes, both from tests sharing files on disk. This unblocks #6176.
DuckLake. Once a process loads DuckLake, its DLL stays loaded until the process exits; closing the connection does not unload it. Any later
install_extension("ducklake")in that process replaces the file on disk, which Windows refuses withCould not move file: Access is denied. Guarding the install did not help: in run 41899285609,load_extensionin the second DuckLake test failed withFailed to create directory "C:\Users\runneradmin\.duckdb": Cannot create a file when that file already exists, even though the first test had just installed DuckLake there, and the install fallback then hit the loaded DLL. So the tests no longer install at all.LOADauto-installs DuckLake when it is missing (local runs, Linux, macOS), and the Windows job installs it before pytest.The transient
~/.duckdberror itself is not explained. Most likely another xdist worker is writing to the shared~/.duckdbat the same moment. If it comes back, it now surfaces as that error instead of "Access is denied".Sushi cache. The two linter tests wrote
examples/sushi/.cachein the repo, and parallel workers collided on it. They now run on a temp copy.Roughly 40 other tests still construct
Contextdirectly onexamples/sushiand share that cache. This PR only isolates the two that failed. Worth a follow-up if more of them flake on Windows.Test Plan
test_drop_table_ducklake_no_cascade_mixed_case_aliasfailed (Access is denied) andtest_get_range_of_a_key_in_model_block_testing_on_sushifailed (Permission denied on the sushi cache).close()(run 41478055750), aduckdb_extensions().installedcheck (run 41636441149), and load-then-install (run 41899285609). The sushi cache failure did not return after the first commit. The installed-check code also passed once (run 41750858262), so one green run is not conclusive on its own.duck_connintests/conftest.pyto main.load_extension("ducklake")against an emptyextension_directoryauto-installs it. Linux and macOS can't reproduce the Windows failure.Merge danger: two-way door, revert the commits. Blast radius is CI only; these are test and CI-workflow changes, and production engines are unchanged.
Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO