Skip to content

Improve query cancellation on control-c#1687

Merged
rolandwalker merged 1 commit intomainfrom
RW/improve-keyboard-interrupt-handling
Mar 8, 2026
Merged

Improve query cancellation on control-c#1687
rolandwalker merged 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 .

@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
 * 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 0674876 to 740ba46 Compare March 7, 2026 23:59
@rolandwalker rolandwalker merged commit 723a79d into main Mar 8, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/improve-keyboard-interrupt-handling branch March 8, 2026 00:02
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.

2 participants