Skip to content

Improve query cancellation on control-c#1687

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/improve-keyboard-interrupt-handling
Open

Improve query cancellation on control-c#1687
rolandwalker wants to merge 1 commit intomainfrom
RW/improve-keyboard-interrupt-handling

Conversation

@rolandwalker
Copy link
Contributor

Description

  • sqlexecute.run() no longer returns a tuple, but a Generator of SQLResults
  • move sqlexecute.connect() within a try block
  • clarify inner error as e2
  • grammar and spelling in commentary
  • if raising a CommandNotFound(), show the command (this can be part of the relevant backtrace)

Due to the first and second bullet, an interrupted query would indeed be cancelled, but the user would at minimum receive poor feedback, and the mycli session could also end.

It might also be desirable to tell click not to handle KeyboardInterrupt.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

 * sqlexecute.run() no longer returns a tuple, but a Generator of
   SQLResults
 * move sqlexecute.connect() within a try block
 * clarify inner error as e2
 * grammar and spelling in commentary
 * if raising a CommandNotFound(), show the command (this can be part
   of the relevant backtrace)

Due to the first and second bullet, an interrupted query would indeed be
cancelled, but the user would at minimum receive poor feedback, and
the mycli session could also end.

It might also be desirable to tell click not to handle
KeyboardInterrupt.
@rolandwalker rolandwalker force-pushed the RW/improve-keyboard-interrupt-handling branch from 0671599 to 0674876 Compare March 7, 2026 17:02
@github-actions
Copy link

github-actions bot commented Mar 7, 2026

No correctness or security regressions stood out in the PR diff itself.

Findings:

  1. Missing test coverage for the new Ctrl-C cancellation flow in mycli/main.py:1212.
    Add an integration test that simulates KeyboardInterrupt during query execution and asserts:
  • sqlexecute.connect() is attempted inside the handler,
  • kill <connection_id> is issued,
  • user-facing feedback is emitted for both success and failure paths,
  • the session remains usable after interruption.
  1. Missing test coverage for the new command text in CommandNotFound at mycli/packages/special/main.py:136.
    Add a unit test for special.execute() with an unknown command to verify the exception message includes the command name and doesn’t alter normal SQL fallback behavior through SQLExecute.run().

Residual risk:

  • The PR changes error/interrupt handling behavior but adds no tests, so regressions are most likely in interrupt edge paths rather than normal execution.

@rolandwalker rolandwalker self-assigned this Mar 7, 2026
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.

1 participant