Skip to content

fix: duckdb interrupt #6806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 15, 2025
Merged

fix: duckdb interrupt #6806

merged 5 commits into from
Oct 15, 2025

Conversation

@akshayka
Copy link
Contributor

@akshayka akshayka commented Oct 15, 2025 •
edited
Loading

This change adds more graceful interrupts of DuckDB queries executed through mo.sql().

Interrupting a connection with a SIGINT can apparently leave the connection in a bad state. This change instead interrupts the connection with its interrupt() method. To enable this, the ExecutionContext is augmented with an additional property in which the runtime can store the active duckdb connection.

This is difficult to test programmatically. Tested manually by interrupting the query in #6787. Basic unit tests are included to ensure the interrupt handler does the right thing.

@akshayka
fix: duckdb interruptions
b181898 
Interrupt duckdb connections with duckdb.interrupt() ahead of raising
a KeyboardInterrupt to avoid leaving connections in a bad state.
@akshayka
Unit tests
2c0887f 
It's difficult to test actually interrupting a running duckdb query.
Instead we add sanity tests for the interrupt handlers behavior when
a connection is installed.
@akshayka akshayka added the bash-focus Area to focus on during release bug bash label Oct 15, 2025
@vercel
Copy link

vercel bot commented Oct 15, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Oct 15, 2025 10:59pm

@akshayka akshayka marked this pull request as draft October 15, 2025 22:04
@akshayka akshayka changed the title Aka/fix duckdb interrupt fix: duckdb interrupt Oct 15, 2025
dmadisetti
dmadisetti reviewed Oct 15, 2025
View reviewed changes
@akshayka akshayka marked this pull request as ready for review October 15, 2025 22:31
mscolnick
mscolnick previously approved these changes Oct 15, 2025
View reviewed changes
Copy link
Contributor

@mscolnick mscolnick left a comment

Choose a reason for hiding this comment

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

Nice!

@akshayka
Copy link
Contributor Author

Failing in CI in a perplexing way ...

FAILED tests/_runtime/test_runtime.py::TestExecution::test_set_ui_element_value_with_cell_run[k] - KeyError: 'defs'
FAILED tests/_runtime/test_runtime.py::TestExecution::test_set_ui_element_value_with_cell_run[strict_kernel] - KeyError: 'defs'
FAILED tests/_runtime/test_runtime.py::TestExecution::test_set_ui_element_value_with_cell_run[lazy_kernel] - KeyError: 'defs'

@akshayka akshayka merged commit 970dbdd into main Oct 15, 2025
29 of 41 checks passed
@akshayka akshayka deleted the aka/fix-duckdb-interrupt branch October 15, 2025 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmadisetti dmadisetti dmadisetti left review comments

@mscolnick mscolnick mscolnick left review comments

@Light2Dark Light2Dark Awaiting requested review from Light2Dark Light2Dark is a code owner

Assignees

No one assigned

Labels

bash-focus Area to focus on during release bug bash

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@akshayka @dmadisetti @mscolnick