diff --git a/changelog.md b/changelog.md index 09df413bf..cbe78be80 100644 --- a/changelog.md +++ b/changelog.md @@ -4,6 +4,7 @@ Upcoming (TBD) Bugfixes --------- * Avoid an exception when exiting while completions are being refreshed. +* Avoid a completions exception when changing to a nonexistent database. Documentation diff --git a/mycli/client_query.py b/mycli/client_query.py index 664620569..1e563dd3b 100644 --- a/mycli/client_query.py +++ b/mycli/client_query.py @@ -40,6 +40,7 @@ def refresh_completions(self, reset: bool = False) -> list[SQLResult]: assert self.sqlexecute is not None if reset: + self.completion_refresher.stop() # Update the active completer's current-schema pointer right # away so unqualified completions reflect a schema switch # even before the background refresh finishes. diff --git a/mycli/completion_refresher.py b/mycli/completion_refresher.py index 32875b9d9..305def95d 100644 --- a/mycli/completion_refresher.py +++ b/mycli/completion_refresher.py @@ -3,6 +3,7 @@ from typing import Callable import pymysql +from pymysql.constants.ER import BAD_DB_ERROR from mycli.packages.special.main import COMMANDS from mycli.packages.sqlresult import SQLResult @@ -146,6 +147,9 @@ def _bg_refresh( if not self._stop_refresh.is_set(): for callback in callbacks: callback(completer) + except pymysql.err.OperationalError as error: + if not self._stop_refresh.is_set() and error.args[0] != BAD_DB_ERROR: + raise except Exception: if not self._stop_refresh.is_set(): raise diff --git a/mycli/main_modes/repl.py b/mycli/main_modes/repl.py index 84dc07ac0..e061d968f 100644 --- a/mycli/main_modes/repl.py +++ b/mycli/main_modes/repl.py @@ -742,6 +742,10 @@ def _one_iteration( mycli.echo('Wise choice!') return + dropping_active_database = is_dropping_database(text, sqlexecute.dbname) + if dropping_active_database: + mycli.completion_refresher.stop() + successful = False try: mycli.logger.debug('sql: %r', text) @@ -870,13 +874,15 @@ def _one_iteration( fg='yellow', ) - if is_dropping_database(text, sqlexecute.dbname): + if dropping_active_database: sqlexecute.dbname = None sqlexecute.connect() if need_completion_refresh(text): - mycli.refresh_completions(reset=need_completion_reset(text)) + mycli.refresh_completions(reset=dropping_active_database or need_completion_reset(text)) finally: + if dropping_active_database and not successful: + mycli.refresh_completions() if mycli.logfile is False: mycli.echo('Warning: This query was not logged.', err=True, fg='red') diff --git a/test/pytests/test_client_query.py b/test/pytests/test_client_query.py index 28bc82191..8adfa5919 100644 --- a/test/pytests/test_client_query.py +++ b/test/pytests/test_client_query.py @@ -15,6 +15,7 @@ def make_refresh_cli() -> tuple[Any, dict[str, Any]]: cli = make_bare_mycli() state: dict[str, Any] = { 'stopped': [], + 'completion_stopped': [], 'refresh_calls': [], 'set_dbname_calls': [], } @@ -30,7 +31,8 @@ def make_refresh_cli() -> tuple[Any, dict[str, Any]]: ) cli.main_formatter = SimpleNamespace(supported_formats=['ascii', 'csv']) cli.completion_refresher = SimpleNamespace( - refresh=lambda executor, callbacks, options: state['refresh_calls'].append((executor, callbacks, options)) + stop=lambda: state['completion_stopped'].append(True), + refresh=lambda executor, callbacks, options: state['refresh_calls'].append((executor, callbacks, options)), ) cli.smart_completion = True state['callback'] = callback @@ -79,6 +81,15 @@ def test_refresh_completions_does_not_update_dbname_without_reset() -> None: main.MyCli.refresh_completions(cli) assert state['set_dbname_calls'] == [] + assert state['completion_stopped'] == [] + + +def test_refresh_completions_stops_completion_worker_when_reset() -> None: + cli, state = make_refresh_cli() + + main.MyCli.refresh_completions(cli, reset=True) + + assert state['completion_stopped'] == [True] def test_refresh_completions_updates_dbname_when_reset() -> None: @@ -93,7 +104,7 @@ def test_refresh_completions_updates_dbname_when_reset() -> None: set_dbname=lambda dbname: set_dbname_calls.append(dbname), ) cli.main_formatter = SimpleNamespace(supported_formats=['table']) - cli.completion_refresher = SimpleNamespace(refresh=lambda executor, callbacks, options: None) + cli.completion_refresher = SimpleNamespace(stop=lambda: None, refresh=lambda executor, callbacks, options: None) main.MyCli.refresh_completions(cli, reset=True) @@ -113,7 +124,7 @@ def test_refresh_completions_uses_lock_when_reset() -> None: set_dbname=lambda dbname: None, ) cli.main_formatter = SimpleNamespace(supported_formats=['table']) - cli.completion_refresher = SimpleNamespace(refresh=lambda executor, callbacks, options: None) + cli.completion_refresher = SimpleNamespace(stop=lambda: None, refresh=lambda executor, callbacks, options: None) main.MyCli.refresh_completions(cli, reset=True) diff --git a/test/pytests/test_completion_refresher.py b/test/pytests/test_completion_refresher.py index 41340e5ea..03795c02d 100644 --- a/test/pytests/test_completion_refresher.py +++ b/test/pytests/test_completion_refresher.py @@ -593,6 +593,22 @@ def stop_refresh(completer, active_executor) -> None: executor.close.assert_called_once_with() +def test_bg_refresh_suppresses_non_operational_error_during_stop(monkeypatch, refresher) -> None: + executor = Mock() + + def stop_with_error(completer, active_executor) -> None: + refresher._stop_refresh.set() + raise RuntimeError('cancelled refresh') + + monkeypatch.setattr(completion_refresher, 'SQLCompleter', Mock()) + monkeypatch.setattr(completion_refresher, 'SQLExecute', Mock(return_value=executor)) + refresher.refreshers = {'stop': stop_with_error} + + refresher._bg_refresh(make_sqlexecute(), Mock(), {}) + + executor.close.assert_called_once_with() + + def test_bg_refresh_skips_callbacks_when_stopped_after_refresh(monkeypatch, refresher) -> None: callback = Mock() executor = Mock() @@ -625,6 +641,28 @@ def fail_refresh(completer, active_executor) -> None: executor.close.assert_called_once_with() +@pytest.mark.parametrize('error_code', [completion_refresher.BAD_DB_ERROR, 2003]) +def test_bg_refresh_only_suppresses_stale_database_error(monkeypatch, refresher, error_code) -> None: + executor = Mock() + callback = Mock() + + def fail_refresh(completer, active_executor) -> None: + raise completion_refresher.pymysql.err.OperationalError(error_code, 'metadata failed') + + monkeypatch.setattr(completion_refresher, 'SQLCompleter', Mock()) + monkeypatch.setattr(completion_refresher, 'SQLExecute', Mock(return_value=executor)) + refresher.refreshers = {'fail': fail_refresh} + + if error_code == completion_refresher.BAD_DB_ERROR: + refresher._bg_refresh(make_sqlexecute(), callback, {}) + else: + with pytest.raises(completion_refresher.pymysql.err.OperationalError, match='metadata failed'): + refresher._bg_refresh(make_sqlexecute(), callback, {}) + + callback.assert_not_called() + executor.close.assert_called_once_with() + + @pytest.mark.parametrize('stopping', [False, True]) def test_bg_refresh_only_suppresses_executor_close_error_when_stopping(monkeypatch, refresher, stopping) -> None: executor = Mock() diff --git a/test/pytests/test_main_modes_repl.py b/test/pytests/test_main_modes_repl.py index c212445c4..401cab92a 100644 --- a/test/pytests/test_main_modes_repl.py +++ b/test/pytests/test_main_modes_repl.py @@ -193,6 +193,7 @@ def make_repl_cli(sqlexecute: Any | None = None) -> Any: timing_calls: list[tuple[str, bool]] = [] log_queries: list[str] = [] cli.refresh_calls = refresh_calls + cli.completion_stop_calls = [] cli.output_calls = output_calls cli.echo_calls = echo_calls cli.timing_calls = timing_calls @@ -208,6 +209,7 @@ def refresh_completions(reset: bool = False) -> list[SQLResult]: return [SQLResult(status='refresh')] cli.refresh_completions = refresh_completions + cli.completion_refresher = SimpleNamespace(stop=lambda: cli.completion_stop_calls.append(True)) def output_timing(timing: str, is_warnings_style: bool = False) -> None: cli.timing_calls.append((timing, is_warnings_style)) @@ -1162,6 +1164,7 @@ def run(self, text: str) -> Iterator[SQLResult]: sqlexecute = FakeSQLExecute() cli = make_repl_cli(sqlexecute) + cli.completion_refresher = SimpleNamespace(stop=lambda: sqlexecute.calls.append('stop')) cli.logfile = False cli.destructive_warning = True monkeypatch.setattr(repl_mode, 'is_redirect_command', lambda text: text == 'redirect') @@ -1185,7 +1188,7 @@ def run(self, text: str) -> Iterator[SQLResult]: assert cli.query_history[-1].successful is True assert cli.query_history[-1].mutating is True assert sqlexecute.dbname is None - assert sqlexecute.calls == ['dropdb', 'connect'] + assert sqlexecute.calls == ['stop', 'dropdb', 'connect'] assert 'Warning: This query was not logged.' in cli.echo_calls repl_mode._one_iteration(cli, repl_mode.ReplState(), 'approved') @@ -1195,6 +1198,26 @@ def run(self, text: str) -> Iterator[SQLResult]: assert 'Wise choice!' in cli.echo_calls +def test_one_iteration_restarts_completions_when_active_database_drop_fails(monkeypatch: pytest.MonkeyPatch) -> None: + patch_repl_runtime_defaults(monkeypatch) + + class FakeSQLExecute: + dbname = 'db' + connection_id = 0 + + def run(self, text: str) -> Iterator[SQLResult]: + raise pymysql.OperationalError(1064, 'drop failed') + + cli = make_repl_cli(FakeSQLExecute()) + monkeypatch.setattr(repl_mode, 'is_dropping_database', lambda text, dbname: True) + + repl_mode._one_iteration(cli, repl_mode.ReplState(), 'drop database db') + + assert cli.completion_stop_calls == [True] + assert cli.refresh_calls == [False] + assert cli.sqlexecute.dbname == 'db' + + @pytest.mark.parametrize( ('terminator', 'setter_name'), [(r'\x', 'set_explorer_output'), (r'\G', 'set_expanded_output')],