From d7041fb404559676d5ad703f13ba699079b6429a Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Sat, 20 Jun 2026 14:09:12 -0400 Subject: [PATCH] make batch execution noninteractive by default * replace --noninteractive flag with --batch-warn, with an inverted meaning * when --batch-warn is set, it overrides myclirc's destructive_warning setting * let --execute work exactly like --batch * send destructive-query confirmation prompts to the standard error, since users running batch scripts will often have the standard output redirected This is a breaking change, not least because the --noninteractive flag no longer has an effect, and disappears from the docs. But passing it causes a Click warning referring the user to --batch-warn. Preparation for release 2.0. --- changelog.md | 5 ++ mycli/main.py | 8 ++- mycli/main_modes/batch.py | 9 ++-- mycli/main_modes/execute.py | 18 ++++++- mycli/packages/interactive_utils.py | 2 +- test/pytests/test_interactive_utils.py | 4 +- test/pytests/test_main.py | 2 + test/pytests/test_main_modes_batch.py | 8 +-- test/pytests/test_main_modes_execute.py | 70 +++++++++++++++++++++++++ 9 files changed, 111 insertions(+), 15 deletions(-) diff --git a/changelog.md b/changelog.md index 585fb5bcd..93657ca92 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,11 @@ Upcoming (TBD) ============== +Breaking Changes +--------- +* Make `--batch` and `--execute` non-interactive by default. + + Internal --------- * Improve test coverage for DSN variable expansion. diff --git a/mycli/main.py b/mycli/main.py index 6bd1b2532..c26ab80bb 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -254,6 +254,10 @@ class CliArgs: clickdc=None, help='Warn before running a destructive query.', ) + batch_warn: bool = clickdc.option( + is_flag=True, + help='Warn before running a destructive query when executing a script.', + ) local_infile: bool | None = clickdc.option( type=bool, is_flag=False, @@ -289,9 +293,11 @@ class CliArgs: type=str, help='SQL script to execute in batch mode.', ) + # deprecated 2026-06-20 noninteractive: bool = clickdc.option( is_flag=True, - help="Don't prompt during batch input. Recommended.", + hidden=True, + deprecated='See --batch-warn.', ) format: str | None = clickdc.option( type=click.Choice(['default', 'csv', 'tsv', 'table']), diff --git a/mycli/main_modes/batch.py b/mycli/main_modes/batch.py index b009486bd..f4c23e469 100644 --- a/mycli/main_modes/batch.py +++ b/mycli/main_modes/batch.py @@ -114,17 +114,16 @@ def dispatch_batch_statements( else: mycli.main_formatter.format_name = 'tsv' - warn_confirmed: bool | None = True - if not cli_args.noninteractive and mycli.destructive_warning and is_destructive(mycli.destructive_keywords, statements): + execution_confirmed: bool | None = True + if cli_args.batch_warn and is_destructive(mycli.destructive_keywords, statements): try: # this seems to work, even though we are reading from stdin above sys.stdin = open('/dev/tty') - # bug: the prompt will not be visible if stdout is redirected - warn_confirmed = confirm_destructive_query(mycli.destructive_keywords, statements) + execution_confirmed = confirm_destructive_query(mycli.destructive_keywords, statements) except (IOError, OSError) as e: mycli.logger.warning('Unable to open TTY as stdin.') raise e - if warn_confirmed: + if execution_confirmed: if cli_args.throttle > 0 and batch_counter >= 1: time.sleep(cli_args.throttle) mycli.run_query(statements, checkpoint=cli_args.checkpoint, new_line=True) diff --git a/mycli/main_modes/execute.py b/mycli/main_modes/execute.py index 9de20df3a..b5d0ec698 100644 --- a/mycli/main_modes/execute.py +++ b/mycli/main_modes/execute.py @@ -5,6 +5,9 @@ import click +from mycli.packages.interactive_utils import confirm_destructive_query +from mycli.packages.sql_utils import is_destructive + if TYPE_CHECKING: from mycli.client import MyCli from mycli.main import CliArgs @@ -34,8 +37,19 @@ def main_execute_from_cli(mycli: 'MyCli', cli_args: 'CliArgs') -> int: else: mycli.main_formatter.format_name = 'tsv' - mycli.run_query(execute_sql, checkpoint=cli_args.checkpoint) - return 0 + execution_confirmed: bool | None = True + if cli_args.batch_warn and is_destructive(mycli.destructive_keywords, execute_sql): + try: + sys.stdin = open('/dev/tty') + execution_confirmed = confirm_destructive_query(mycli.destructive_keywords, execute_sql) + except (IOError, OSError) as e: + mycli.logger.warning('Unable to open TTY as stdin.') + raise e + if execution_confirmed: + mycli.run_query(execute_sql, checkpoint=cli_args.checkpoint) + return 0 + else: + return 1 except Exception as e: click.secho(str(e), err=True, fg="red") return 1 diff --git a/mycli/packages/interactive_utils.py b/mycli/packages/interactive_utils.py index fa0f05378..e81ef1328 100644 --- a/mycli/packages/interactive_utils.py +++ b/mycli/packages/interactive_utils.py @@ -36,7 +36,7 @@ def confirm_destructive_query(keywords: list[str], queries: str) -> bool | None: """ prompt_text = "You're about to run a destructive command.\nDo you want to proceed? (y/n)" if is_destructive(keywords, queries) and sys.stdin.isatty(): - return prompt(prompt_text, type=BOOLEAN_TYPE) + return prompt(prompt_text, type=BOOLEAN_TYPE, err=True) else: return None diff --git a/test/pytests/test_interactive_utils.py b/test/pytests/test_interactive_utils.py index 66182c938..3b28ca8dc 100644 --- a/test/pytests/test_interactive_utils.py +++ b/test/pytests/test_interactive_utils.py @@ -92,7 +92,7 @@ def fake_is_destructive(keywords: list[str], query: str) -> bool: assert prompt_calls == [ ( ("You're about to run a destructive command.\nDo you want to proceed? (y/n)",), - {'type': interactive_utils.BOOLEAN_TYPE}, + {'type': interactive_utils.BOOLEAN_TYPE, 'err': True}, ) ] @@ -120,7 +120,7 @@ def fake_is_destructive(keywords: list[str], query: str) -> bool: assert prompt_calls == [ ( ("You're about to run a destructive command.\nDo you want to proceed? (y/n)",), - {'type': interactive_utils.BOOLEAN_TYPE}, + {'type': interactive_utils.BOOLEAN_TYPE, 'err': True}, ) ] diff --git a/test/pytests/test_main.py b/test/pytests/test_main.py index b16d3495b..0b262a347 100644 --- a/test/pytests/test_main.py +++ b/test/pytests/test_main.py @@ -828,6 +828,8 @@ def test_help_strings_end_with_periods(): """Make sure click options have help text that end with a period.""" for param in click_entrypoint.params: if isinstance(param, click.core.Option): + if param.hidden: + continue assert hasattr(param, "help") assert param.help.endswith(".") diff --git a/test/pytests/test_main_modes_batch.py b/test/pytests/test_main_modes_batch.py index 250e462a4..bf4ce66c2 100644 --- a/test/pytests/test_main_modes_batch.py +++ b/test/pytests/test_main_modes_batch.py @@ -24,7 +24,7 @@ @dataclass class DummyCliArgs: format: str = 'tsv' - noninteractive: bool = True + batch_warn: bool = False throttle: float = 0.0 checkpoint: str | TextIOWrapper | None = None batch: str | None = None @@ -428,7 +428,7 @@ def test_dispatch_batch_statements_sets_expected_output_format( def test_dispatch_batch_statements_confirms_destructive_queries_before_running(monkeypatch) -> None: mycli = DummyMyCli(destructive_warning=True) - cli_args = DummyCliArgs(noninteractive=False) + cli_args = DummyCliArgs(batch_warn=True) opened_tty = object() monkeypatch.setattr(batch_mode, 'is_destructive', lambda _keywords, _statement: True) @@ -444,7 +444,7 @@ def test_dispatch_batch_statements_confirms_destructive_queries_before_running(m def test_dispatch_batch_statements_skips_query_when_destructive_confirmation_is_rejected(monkeypatch) -> None: mycli = DummyMyCli(destructive_warning=True) - cli_args = DummyCliArgs(noninteractive=False) + cli_args = DummyCliArgs(batch_warn=True) monkeypatch.setattr(batch_mode, 'is_destructive', lambda _keywords, _statement: True) monkeypatch.setattr(batch_mode, 'confirm_destructive_query', lambda _keywords, _statement: False) @@ -458,7 +458,7 @@ def test_dispatch_batch_statements_skips_query_when_destructive_confirmation_is_ def test_dispatch_batch_statements_raises_when_tty_cannot_be_opened(monkeypatch) -> None: mycli = DummyMyCli(destructive_warning=True) - cli_args = DummyCliArgs(noninteractive=False) + cli_args = DummyCliArgs(batch_warn=True) monkeypatch.setattr(batch_mode, 'is_destructive', lambda _keywords, _statement: True) monkeypatch.setattr(batch_mode, 'open', lambda _path: (_ for _ in ()).throw(OSError('tty unavailable')), raising=False) diff --git a/test/pytests/test_main_modes_execute.py b/test/pytests/test_main_modes_execute.py index 2b36fe31e..ca1fe9296 100644 --- a/test/pytests/test_main_modes_execute.py +++ b/test/pytests/test_main_modes_execute.py @@ -1,5 +1,6 @@ from __future__ import annotations +import builtins from dataclasses import dataclass from types import SimpleNamespace from typing import Any, cast @@ -14,6 +15,7 @@ class DummyCliArgs: execute: str | None format: str = 'tsv' batch: str | None = None + batch_warn: bool = False checkpoint: str | None = None @@ -22,11 +24,21 @@ class DummyFormatter: format_name: str | None = None +class DummyLogger: + def __init__(self) -> None: + self.warning_calls: list[str] = [] + + def warning(self, message: str) -> None: + self.warning_calls.append(message) + + class DummyMyCli: def __init__(self, run_query_error: Exception | None = None) -> None: self.main_formatter = DummyFormatter() self.run_query_error = run_query_error self.ran_queries: list[tuple[str, str | None]] = [] + self.destructive_keywords = ['drop'] + self.logger = DummyLogger() def run_query(self, query: str, checkpoint: str | None = None) -> None: if self.run_query_error is not None: @@ -125,3 +137,61 @@ def test_main_execute_from_cli_reports_query_errors(monkeypatch) -> None: assert mycli.main_formatter.format_name == 'ascii' assert mycli.ran_queries == [] assert secho_calls == [('boom', True, 'red')] + + +def test_main_execute_from_cli_confirms_destructive_query(monkeypatch) -> None: + mycli = DummyMyCli() + tty = object() + confirm_calls: list[tuple[list[str], str]] = [] + + monkeypatch.setattr(execute_mode, 'sys', fake_sys(stdin_tty=True)) + monkeypatch.setattr(execute_mode, 'is_destructive', lambda keywords, query: True) + monkeypatch.setattr(builtins, 'open', lambda path: tty) + + def confirm_destructive_query(keywords: list[str], query: str) -> bool: + confirm_calls.append((keywords, query)) + return True + + monkeypatch.setattr(execute_mode, 'confirm_destructive_query', confirm_destructive_query) + + result = main_execute_from_cli(mycli, DummyCliArgs(execute='drop table t', batch_warn=True)) + + assert result == 0 + assert execute_mode.sys.stdin is tty + assert confirm_calls == [(['drop'], 'drop table t')] + assert mycli.ran_queries == [('drop table t', None)] + + +def test_main_execute_from_cli_returns_error_when_destructive_query_is_rejected(monkeypatch) -> None: + mycli = DummyMyCli() + + monkeypatch.setattr(execute_mode, 'sys', fake_sys(stdin_tty=True)) + monkeypatch.setattr(execute_mode, 'is_destructive', lambda keywords, query: True) + monkeypatch.setattr(builtins, 'open', lambda path: object()) + monkeypatch.setattr(execute_mode, 'confirm_destructive_query', lambda keywords, query: False) + + result = main_execute_from_cli(mycli, DummyCliArgs(execute='drop table t', batch_warn=True)) + + assert result == 1 + assert mycli.ran_queries == [] + + +def test_main_execute_from_cli_reports_tty_open_error_for_destructive_query(monkeypatch) -> None: + secho_calls: list[tuple[str, bool, str]] = [] + mycli = DummyMyCli() + + monkeypatch.setattr(execute_mode, 'sys', fake_sys(stdin_tty=True)) + monkeypatch.setattr(execute_mode, 'is_destructive', lambda keywords, query: True) + monkeypatch.setattr(builtins, 'open', lambda path: (_ for _ in ()).throw(OSError('no tty'))) + monkeypatch.setattr( + execute_mode.click, + 'secho', + lambda message, err, fg: secho_calls.append((message, err, fg)), + ) + + result = main_execute_from_cli(mycli, DummyCliArgs(execute='drop table t', batch_warn=True)) + + assert result == 1 + assert mycli.logger.warning_calls == ['Unable to open TTY as stdin.'] + assert mycli.ran_queries == [] + assert secho_calls == [('no tty', True, 'red')]