sqlite3: fix closed connection error message to match CPython - #8362
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe SQLite connection lock path now distinguishes an uninitialized connection from a connection that was initialized and subsequently closed, reporting a dedicated ChangesSQLite error handling
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When a Connection is explicitly closed via con.close(), subsequent operations (cursor(), commit(), rollback(), create_function(), etc.) should raise ProgrammingError with 'Cannot operate on a closed database.' to match CPython behaviour. Previously, _db_lock() always returned 'Base Connection.__init__ not called.' when self.db was None, without distinguishing between a connection that was never initialised (subclass before __init__) and one that was initialised and then explicitly closed. Fix: inspect the initialized atomic flag — if True but db is None, the connection was closed; if False, it was never initialised. Assisted-by: GitHub Copilot:claude-sonnet-4-6
a9012f5 to
e1ca6c9
Compare
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/sqlite3 dependencies:
dependent tests: (2 tests)
Legend:
|
There was a problem hiding this comment.
Pull request overview
Aligns RustPython’s _sqlite3.Connection behavior with CPython by distinguishing “never initialized” connections (e.g., subclass where __init__ wasn’t called) from connections that were successfully initialized and later explicitly closed, so post-close operations raise the correct ProgrammingError message.
Changes:
- Updates
Connection::_db_lock()to raiseProgrammingError: Cannot operate on a closed database.whendbisNoneand the connection has previously been initialized. - Keeps the existing
ProgrammingError: Base Connection.__init__ not called.for truly uninitialized subclass instances. - Removes
@unittest.expectedFailuremarkers fromClosedConTestsnow that the error message matches CPython.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Lib/test/test_sqlite3/test_dbapi.py |
Unmarks previously-expected failures for closed-connection message checks, making the CPython-aligned behavior required by tests. |
crates/stdlib/src/_sqlite3.rs |
Adjusts _db_lock() to select the CPython-matching closed-connection error message based on the initialized flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When a Connection is explicitly closed via con.close(), subsequent operations (cursor(), commit(), rollback(), create_function(), etc.) should raise ProgrammingError with 'Cannot operate on a closed database.' to match CPython behaviour.
Previously, _db_lock() always returned 'Base Connection.init not called.' when self.db was None, without distinguishing between a connection that was never initialised (subclass before init) and one that was initialised and then explicitly closed.
Assisted-by: GitHub Copilot:claude-sonnet-4-6
Summary
Summary by CodeRabbit