Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions mycli/client_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions mycli/completion_refresher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions mycli/main_modes/repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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')

Expand Down
17 changes: 14 additions & 3 deletions test/pytests/test_client_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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': [],
}
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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)

Expand All @@ -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)

Expand Down
38 changes: 38 additions & 0 deletions test/pytests/test_completion_refresher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
25 changes: 24 additions & 1 deletion test/pytests/test_main_modes_repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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')],
Expand Down
Loading