Skip to content

Move --batch branches out of main.py#1769

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/move-batch-branches-out-of-main-py
Open

Move --batch branches out of main.py#1769
rolandwalker wants to merge 1 commit intomainfrom
RW/move-batch-branches-out-of-main-py

Conversation

@rolandwalker
Copy link
Copy Markdown
Contributor

Description

Move --batch branches out of main.py, creating a main_modes directory, into which other modes can be moved.

It might be considered very rough to pass the entire MyCli and CliArgs instances as arguments, but this is at least a step to breaking up the large main.py file into logical sections.

Some comments regarding statement count were removed, and exceptions are handled differently, but no functional change is intended.

Relevant tests are moved to test/pytests/test_main_modes_batch.py, and new tests are added for greater coverage.

main_modes/ might not be the best name. main_dispatch? I didn't want to write out main_execution_paths.

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 self-assigned this Apr 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Codex Review

  1. High: unhandled query/runtime exceptions now escape batch mode and can produce traceback instead of controlled CLI exit
    In mycli/main_modes/batch.py, dispatch_batch_statements() catches Exception, prints, then re-raises. But callers only catch narrow exception sets:

    A RuntimeError (or most DB/query exceptions) from run_query() now bypasses these handlers. Before this refactor, the inline code in main.py exited immediately on any exception.
    Action: either catch Exception in each main_batch_* function (and return 1), or stop re-raising in dispatch_batch_statements and return a status consistently.

  2. Medium: destructive-confirmation TTY failures can now be uncaught in stdin batch path
    dispatch_batch_statements() re-raises OSError/IOError when /dev/tty cannot be opened. main_batch_from_stdin() does not catch those (line 138), so stdin-driven batch mode can crash with traceback instead of a clean error exit.
    Action: include OSError|IOError (or Exception) in main_batch_from_stdin() handling.

  3. Missing test coverage for the above regression
    Current tests validate dispatch_batch_statements re-raises (test_dispatch_batch_statements_sleeps_and_reraises_query_errors) but do not assert top-level CLI behavior when dispatch raises non-IO exceptions in each mode.
    Action: add tests that simulate dispatch_batch_statements raising RuntimeError (and TTY OSError) through:

    • main_batch_with_progress_bar
    • main_batch_without_progress_bar
    • main_batch_from_stdin
      and assert clean nonzero exit with error message, no uncaught exception.

Could not run the test suite locally because uv is not installed in this environment (uv: command not found).

creating a main_modes directory, into which other modes can be moved.

It might be considered very rough to pass the entire MyCli and CliArgs
instances as arguments, but this is at least a step to breaking up the
large main.py file into logical sections.

Some comments regarding statement count were removed, exceptions are
handled differently, and better care is taken to close a filehandle, but
no functional change is intended.

Relevant tests are moved to test/pytests/test_main_modes_batch.py, and
new tests are added for greater coverage.
@rolandwalker rolandwalker force-pushed the RW/move-batch-branches-out-of-main-py branch from 4f45b3d to 08ef89b Compare April 1, 2026 16:31
@rolandwalker rolandwalker mentioned this pull request Apr 1, 2026
3 tasks
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