Skip to content

fix(python): close SQLite connections in BatchUDFCheckpoint#5733

Merged
wjones127 merged 1 commit intolance-format:mainfrom
wjones127:ci/windows-fix
Jan 16, 2026
Merged

fix(python): close SQLite connections in BatchUDFCheckpoint#5733
wjones127 merged 1 commit intolance-format:mainfrom
wjones127:ci/windows-fix

Conversation

@wjones127
Copy link
Copy Markdown
Contributor

@wjones127 wjones127 commented Jan 16, 2026

Summary

  • Fix Windows CI failure by properly closing SQLite connections using context managers
  • Connections in BatchUDFCheckpoint were not being explicitly closed, causing file lock issues when cleanup() tried to delete the checkpoint file

Fixes #5732

Test plan

  • Existing test test_add_columns_udf_caching passes locally
  • Windows CI passes

🤖 Generated with Claude Code

@github-actions github-actions Bot added bug Something isn't working python labels Jan 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Code Review

Overall: Clean fix for the Windows CI issue. Using context managers to ensure SQLite connections are properly closed is the right approach.

P0/P1 Issues: None found

The change correctly addresses the file lock issue on Windows by ensuring connections are closed before cleanup() attempts to delete the file.

Minor note (not blocking)

The with sqlite3.connect(path) as conn: pattern commits on exit but doesn't explicitly close the connection—it just commits the transaction. However, in practice this works fine because:

  1. The connection object goes out of scope immediately after the with block
  2. CPython's reference counting typically closes it immediately
  3. This pattern is idiomatic Python for SQLite

If you wanted to be extra explicit, you could use:

conn = sqlite3.connect(path)
try:
    # ... operations ...
    conn.commit()
finally:
    conn.close()

But the current approach should work correctly and is more concise.

LGTM ✓

SQLite connections were not being properly closed, causing file lock
issues on Windows. The cleanup() method could not delete the checkpoint
file because connections were still holding handles.

The sqlite3 context manager only handles transactions (commit/rollback),
not connection closing. Using contextlib.closing() ensures connections
are actually closed when exiting the with block.

Fixes lance-format#5732

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wjones127 wjones127 marked this pull request as ready for review January 16, 2026 21:33
Copy link
Copy Markdown
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for helping with the CI 🎉

@wjones127 wjones127 merged commit b0f8d23 into lance-format:main Jan 16, 2026
15 checks passed
jackye1995 pushed a commit to jackye1995/lance that referenced this pull request Jan 21, 2026
…rmat#5733)

## Summary
- Fix Windows CI failure by properly closing SQLite connections using
context managers
- Connections in `BatchUDFCheckpoint` were not being explicitly closed,
causing file lock issues when `cleanup()` tried to delete the checkpoint
file

Fixes lance-format#5732

## Test plan
- [x] Existing test `test_add_columns_udf_caching` passes locally
- [x] Windows CI passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
vivek-bharathan pushed a commit to vivek-bharathan/lance that referenced this pull request Feb 2, 2026
…rmat#5733)

## Summary
- Fix Windows CI failure by properly closing SQLite connections using
context managers
- Connections in `BatchUDFCheckpoint` were not being explicitly closed,
causing file lock issues when `cleanup()` tried to delete the checkpoint
file

Fixes lance-format#5732

## Test plan
- [x] Existing test `test_add_columns_udf_caching` passes locally
- [x] Windows CI passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python Windows CI failure: test_schema_evolution.py::test_add_columns_udf_caching

2 participants