From 5bb475d3da3e956009a3097595ed26aa221b964a Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Tue, 15 Aug 2023 23:12:49 -0700 Subject: [PATCH 1/8] improve error when calling sqlcmd --- CHANGELOG.md | 1 + src/sql/magic_cmd.py | 15 ++++++++++++- src/tests/test_magic_cmd.py | 45 +++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f6950357..669f0948f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [Fix] Current connection and switching connections message only displayed when `feedback>=1` * [Fix] `--persist/--persist-replace` perform `ROLLBACK` automatically when needed * [Fix] `ResultSet` footer (when `displaylimit` truncates results and when showing how to convert to a data frame) now appears in the `ResultSet` plain text representation (#682) +* [Fix] Improve error when calling `%sqlcmd` (#761) ## 0.9.1 (2023-08-10) diff --git a/src/sql/magic_cmd.py b/src/sql/magic_cmd.py index d816189b7..ecb144608 100644 --- a/src/sql/magic_cmd.py +++ b/src/sql/magic_cmd.py @@ -11,6 +11,7 @@ from sql.cmd.profile import profile from sql.cmd.explore import explore from sql.cmd.snippets import snippets +from sql.connection import ConnectionManager try: from traitlets.config.configurable import Configurable @@ -51,6 +52,8 @@ def _validate_execute_inputs(self, line): "explore", "snippets", ] + CONNECTION_INDEPENDENT_COMMANDS = ["snippets"] + SUPPORTED_BY_BOTH_COMMANDS = ["profile"] VALID_COMMANDS_MSG = ( f"Missing argument for %sqlcmd. " @@ -64,8 +67,18 @@ def _validate_execute_inputs(self, line): command, others = split[0].strip(), split[1:] if command in AVAILABLE_SQLCMD_COMMANDS: - if command not in ["profile"]: + if ( + command not in CONNECTION_INDEPENDENT_COMMANDS + and not ConnectionManager.current + ): + raise exceptions.RuntimeError("No active connection") + + if command not in [ + *CONNECTION_INDEPENDENT_COMMANDS, + *SUPPORTED_BY_BOTH_COMMANDS, + ]: util.support_only_sql_alchemy_connection("%sqlcmd") + return self.execute(command, others) else: raise exceptions.UsageError( diff --git a/src/tests/test_magic_cmd.py b/src/tests/test_magic_cmd.py index c036b4e94..aa112e334 100644 --- a/src/tests/test_magic_cmd.py +++ b/src/tests/test_magic_cmd.py @@ -125,6 +125,51 @@ def test_error(tmp_empty, ip, cell, error_message): assert str(excinfo.value) == error_message +@pytest.mark.parametrize( + "command", + [ + "tables", + "columns", + "test", + "profile", + "explore", + ], +) +def test_sqlcmd_error_when_no_connection(ip_empty, command): + with pytest.raises(UsageError) as excinfo: + ip_empty.run_cell(f"%sqlcmd {command}") + + assert excinfo.value.error_type == "RuntimeError" + assert str(excinfo.value) == "No active connection" + + +def test_sqlcmd_snippets_when_no_connection(ip_empty, capsys): + ip_empty.run_cell("%sqlcmd snippets") + captured = capsys.readouterr() + assert "No snippets stored" in captured.out + + +@pytest.mark.parametrize( + "query", + [ + ("%sqlcmd tables"), + ("%sqlcmd columns --table penguins.csv"), + ("%sqlcmd test --table penguins.csv --column body_mass_g --greater 2900"), + ("%sqlcmd explore --table penguins.csv"), + ], +) +def test_sqlcmd_not_supported_error(ip_with_connections, query, capsys): + ip_with_connections.run_cell("%sql duckdb_dbapi") + expected_error_message = ( + "%sqlcmd is only supported with SQLAlchemy connections, " + "not with DBAPI connections" + ) + with pytest.raises(UsageError) as excinfo: + ip_with_connections.run_cell(query) + + assert expected_error_message in str(excinfo.value) + + def test_tables(ip): out = ip.run_cell("%sqlcmd tables").result._repr_html_() assert "author" in out From 562cafd02a28ac3490e3df3663d84394e8aa2bd3 Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Wed, 16 Aug 2023 08:34:57 -0700 Subject: [PATCH 2/8] fix --- src/tests/test_magic_cmd.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/test_magic_cmd.py b/src/tests/test_magic_cmd.py index aa112e334..1efefbd68 100644 --- a/src/tests/test_magic_cmd.py +++ b/src/tests/test_magic_cmd.py @@ -144,6 +144,9 @@ def test_sqlcmd_error_when_no_connection(ip_empty, command): def test_sqlcmd_snippets_when_no_connection(ip_empty, capsys): + for key in list(store): + del store[key] + ip_empty.run_cell("%sqlcmd snippets") captured = capsys.readouterr() assert "No snippets stored" in captured.out From a6ab44e2d446d4ea7a0271d1ad11998748bf412c Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Wed, 16 Aug 2023 17:50:35 -0700 Subject: [PATCH 3/8] ci From d06d806890a730b5a9f70d62b78d77006e962508 Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Wed, 16 Aug 2023 18:09:15 -0700 Subject: [PATCH 4/8] ci From d29333df13b70ad2247a5bf10fcd866f5f377c76 Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Thu, 17 Aug 2023 13:00:14 -0700 Subject: [PATCH 5/8] fix --- src/sql/magic_cmd.py | 23 +++++++++++++++-------- src/tests/test_magic_cmd.py | 8 ++++---- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/sql/magic_cmd.py b/src/sql/magic_cmd.py index ecb144608..45c0d3606 100644 --- a/src/sql/magic_cmd.py +++ b/src/sql/magic_cmd.py @@ -52,8 +52,14 @@ def _validate_execute_inputs(self, line): "explore", "snippets", ] - CONNECTION_INDEPENDENT_COMMANDS = ["snippets"] - SUPPORTED_BY_BOTH_COMMANDS = ["profile"] + COMMANDS_CONNECTION_REQUIRED = [ + "tables", + "columns", + "test", + "profile", + "explore", + ] + COMMANDS_SQLALCHEMY_ONLY = ["tables", "columns", "test", "explore"] VALID_COMMANDS_MSG = ( f"Missing argument for %sqlcmd. " @@ -68,15 +74,16 @@ def _validate_execute_inputs(self, line): if command in AVAILABLE_SQLCMD_COMMANDS: if ( - command not in CONNECTION_INDEPENDENT_COMMANDS + command in COMMANDS_CONNECTION_REQUIRED and not ConnectionManager.current ): - raise exceptions.RuntimeError("No active connection") + raise exceptions.RuntimeError( + f"Cannot execute %sqlcmd {command} because there " + "is no active connection. Connect to a database " + "and try again." + ) - if command not in [ - *CONNECTION_INDEPENDENT_COMMANDS, - *SUPPORTED_BY_BOTH_COMMANDS, - ]: + if command in COMMANDS_SQLALCHEMY_ONLY: util.support_only_sql_alchemy_connection("%sqlcmd") return self.execute(command, others) diff --git a/src/tests/test_magic_cmd.py b/src/tests/test_magic_cmd.py index 1efefbd68..aacb18529 100644 --- a/src/tests/test_magic_cmd.py +++ b/src/tests/test_magic_cmd.py @@ -140,13 +140,13 @@ def test_sqlcmd_error_when_no_connection(ip_empty, command): ip_empty.run_cell(f"%sqlcmd {command}") assert excinfo.value.error_type == "RuntimeError" - assert str(excinfo.value) == "No active connection" + assert str(excinfo.value) == ( + f"Cannot execute %sqlcmd {command} because there is no " + "active connection. Connect to a database and try again." + ) def test_sqlcmd_snippets_when_no_connection(ip_empty, capsys): - for key in list(store): - del store[key] - ip_empty.run_cell("%sqlcmd snippets") captured = capsys.readouterr() assert "No snippets stored" in captured.out From cdbcc90f6e2d124cd87527cc8087805e6ceec672 Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Thu, 17 Aug 2023 17:17:53 -0700 Subject: [PATCH 6/8] fix --- src/tests/test_magic_cmd.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/test_magic_cmd.py b/src/tests/test_magic_cmd.py index aacb18529..900ce884f 100644 --- a/src/tests/test_magic_cmd.py +++ b/src/tests/test_magic_cmd.py @@ -147,6 +147,9 @@ def test_sqlcmd_error_when_no_connection(ip_empty, command): def test_sqlcmd_snippets_when_no_connection(ip_empty, capsys): + for key in list(store): + del store[key] + ip_empty.run_cell("%sqlcmd snippets") captured = capsys.readouterr() assert "No snippets stored" in captured.out From 0837cea74307cdd32f409c537442f4aad2d5fe4d Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Thu, 17 Aug 2023 18:43:57 -0700 Subject: [PATCH 7/8] fix --- src/sql/magic_cmd.py | 2 +- src/tests/test_magic_cmd.py | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/sql/magic_cmd.py b/src/sql/magic_cmd.py index 45c0d3606..77bff8ef0 100644 --- a/src/sql/magic_cmd.py +++ b/src/sql/magic_cmd.py @@ -84,7 +84,7 @@ def _validate_execute_inputs(self, line): ) if command in COMMANDS_SQLALCHEMY_ONLY: - util.support_only_sql_alchemy_connection("%sqlcmd") + util.support_only_sql_alchemy_connection(f"%sqlcmd {command}") return self.execute(command, others) else: diff --git a/src/tests/test_magic_cmd.py b/src/tests/test_magic_cmd.py index 900ce884f..cd4cb3d6d 100644 --- a/src/tests/test_magic_cmd.py +++ b/src/tests/test_magic_cmd.py @@ -156,18 +156,21 @@ def test_sqlcmd_snippets_when_no_connection(ip_empty, capsys): @pytest.mark.parametrize( - "query", + "query, command", [ - ("%sqlcmd tables"), - ("%sqlcmd columns --table penguins.csv"), - ("%sqlcmd test --table penguins.csv --column body_mass_g --greater 2900"), - ("%sqlcmd explore --table penguins.csv"), + ("%sqlcmd tables", "tables"), + ("%sqlcmd columns --table penguins.csv", "columns"), + ( + "%sqlcmd test --table penguins.csv --column body_mass_g --greater 2900", + "test", + ), + ("%sqlcmd explore --table penguins.csv", "explore"), ], ) -def test_sqlcmd_not_supported_error(ip_with_connections, query, capsys): +def test_sqlcmd_not_supported_error(ip_with_connections, query, command, capsys): ip_with_connections.run_cell("%sql duckdb_dbapi") expected_error_message = ( - "%sqlcmd is only supported with SQLAlchemy connections, " + f"%sqlcmd {command} is only supported with SQLAlchemy connections, " "not with DBAPI connections" ) with pytest.raises(UsageError) as excinfo: From 97ae242b64fa3301b23561bd88f38b5051f1bfc4 Mon Sep 17 00:00:00 2001 From: SangGyu An Date: Thu, 17 Aug 2023 19:23:51 -0700 Subject: [PATCH 8/8] fix --- src/tests/integration/test_questDB.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tests/integration/test_questDB.py b/src/tests/integration/test_questDB.py index 306306afe..cea75a899 100644 --- a/src/tests/integration/test_questDB.py +++ b/src/tests/integration/test_questDB.py @@ -615,17 +615,17 @@ def test_sql(ip_questdb, query, expected_results): @pytest.mark.parametrize( - "query", + "query, command", [ - ("%sqlcmd tables"), - ("%sqlcmd tables --schema some_schema"), - ("%sqlcmd columns --table penguins.csv"), - ("%sqlcmd test"), - ("%sqlcmd test --table penguins.csv"), + ("%sqlcmd tables", "tables"), + ("%sqlcmd tables --schema some_schema", "tables"), + ("%sqlcmd columns --table penguins.csv", "columns"), + ("%sqlcmd test", "test"), + ("%sqlcmd test --table penguins.csv", "test"), ], ) -def test_sqlcmd_not_supported_error(ip_questdb, query, capsys): - expected_error_message = f"%sqlcmd {NOT_SUPPORTED_SUFFIX}" +def test_sqlcmd_not_supported_error(ip_questdb, query, command, capsys): + expected_error_message = f"%sqlcmd {command} {NOT_SUPPORTED_SUFFIX}" with pytest.raises(UsageError) as excinfo: ip_questdb.run_cell(query)