From 1b78d72dab532dcf66d356f39c50f9f64a7e4010 Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 1 Oct 2025 15:20:03 +0200 Subject: [PATCH 01/13] #271: Implemented checking SCS content for completeness and correctness --- doc/changes/unreleased.md | 1 + exasol/nb_connector/cli/commands/check.py | 4 ++- .../nb_connector/cli/processing/processing.py | 32 +++++++++++++++++++ test/unit/cli/test_cli.py | 27 ++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 0d14eb05..85b2dbdc 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -12,6 +12,7 @@ Additionally the release includes a CLI for populating the Secure Configuration * #263: Added functions for handling a set of options for SCS CLI * #267: Implemented modify operations for the SCS * #269: Implemented showing SCS content +* #271: Implemented checking SCS content ## Refactorings diff --git a/exasol/nb_connector/cli/commands/check.py b/exasol/nb_connector/cli/commands/check.py index c874d6ec..c0a17501 100644 --- a/exasol/nb_connector/cli/commands/check.py +++ b/exasol/nb_connector/cli/commands/check.py @@ -6,6 +6,7 @@ from exasol.nb_connector.cli.groups import cli from exasol.nb_connector.cli.options import SCS_OPTIONS from exasol.nb_connector.cli.param_wrappers import add_params +from exasol.nb_connector.cli.processing import processing @cli.command() @@ -24,4 +25,5 @@ def check(scs_file: Path, connect: bool): Optionally also verify if a connection to the configured Exasol database instance is successful. """ - pass + result = processing.check_scs(scs_file, connect) + sys.exit(result) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index 6f5f1eda..d90f98cf 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -23,6 +23,9 @@ get_option_set, get_scs, ) +from exasol.nb_connector.connections import open_pyexasol_connection +from exasol.nb_connector.itde_manager import bring_itde_up +from exasol.nb_connector.secret_store import Secrets LOG = logging.getLogger(__name__) @@ -63,6 +66,35 @@ def save( return 0 +def verify_connection(scs: Secrets) -> int: + if BackendSelector(scs).use_itde: + # Question: Is it OK, to let bring_itde_up modify the SCS content, here? + False and bring_itde_up(scs) + report.warning(f"Bring up ITDE currently disabled") + return 1 + try: + open_pyexasol_connection(scs).execute("SELECT 1 FROM DUAL").fetchone() + except Exception as ex: + report.error(f"Failed to connect to the configured database {ex}") + return 1 + report.success("Connection to the configured database instance was successful.") + return 0 + + +def check_scs(scs_file: Path, connect: bool) -> int: + """ + Check the SCS content for completeness. Infer the required keys from + backend and use_itde if these are contained in the SCS already. + + If parameter `connect` is True then also verify if a connection to the + configured Exasol database instance is successful. + """ + options = get_option_set(scs_file) + if not options or not options.check(): + return 1 + return 0 if not connect else verify_connection(options.scs) + + def show_scs_content(scs_file: Path) -> int: """ If the SCS contains a proper backend selection, then show the SCS diff --git a/test/unit/cli/test_cli.py b/test/unit/cli/test_cli.py index 21433ebe..9a8f607c 100644 --- a/test/unit/cli/test_cli.py +++ b/test/unit/cli/test_cli.py @@ -1,6 +1,11 @@ +import getpass import itertools +from typing import Iterator import os from inspect import cleandoc +from pathlib import Path +from test.utils.integration_test_utils import sample_db_file +from unittest.mock import Mock import click import pytest @@ -16,9 +21,15 @@ def assert_error(result: click.testing.Result, message: str): assert message in result.output +def assert_success(result: click.testing.Result, message: str): + assert result.exit_code == 0 + assert message in result.output + + @pytest.mark.parametrize( "args", [ + (commands.check, []), (commands.configure, ["onprem"]), (commands.configure, ["saas"]), (commands.configure, ["docker-db"]), @@ -30,6 +41,20 @@ def test_missing_scs_file(args): assert_error(result, "Error: Missing argument 'SCS_FILE'") +@pytest.fixture +def scs_file() -> Iterator[Path]: + with sample_db_file() as scs_file: + yield scs_file + + +def test_check_ask_for_master_password(monkeypatch, scs_file): + mock = Mock(return_value="interactive password") + monkeypatch.setattr(getpass, "getpass", mock) + result = CliRunner().invoke(commands.check, [str(scs_file)]) + assert mock.called + assert_error(result, f"{scs_file} does not contain any backend") + + @pytest.fixture def scs_with_env(secrets, monkeypatch) -> Secrets: """ @@ -137,5 +162,7 @@ def cmd_args(): monkeypatch.setitem(os.environ, env_var, f"secret {i+1}") result = CliRunner().invoke(commands.configure, cmd_args()) assert result.exit_code == 0 + result = CliRunner().invoke(commands.check, [scs_file]) + assert_success(result, f"Configuration is complete") result = CliRunner().invoke(commands.show, [scs_file]) assert cleandoc(expected_show) in result.output From 56c96bc6261491118c433dae4643f1aeaa9066ba Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 1 Oct 2025 16:55:18 +0200 Subject: [PATCH 02/13] Replaced old-school int return values by exceptions --- exasol/nb_connector/cli/commands/check.py | 9 ++++-- exasol/nb_connector/cli/commands/configure.py | 9 ++---- exasol/nb_connector/cli/commands/show.py | 9 ++++-- .../nb_connector/cli/processing/option_set.py | 14 ++++----- .../nb_connector/cli/processing/processing.py | 31 ++++++++----------- exasol/nb_connector/cli/reporting.py | 2 +- test/unit/cli/test_option_set.py | 25 +++++++-------- test/unit/cli/test_processing_save.py | 3 +- 8 files changed, 51 insertions(+), 51 deletions(-) diff --git a/exasol/nb_connector/cli/commands/check.py b/exasol/nb_connector/cli/commands/check.py index c0a17501..9352ff45 100644 --- a/exasol/nb_connector/cli/commands/check.py +++ b/exasol/nb_connector/cli/commands/check.py @@ -3,10 +3,12 @@ import click +from exasol.nb_connector.cli import reporting as report from exasol.nb_connector.cli.groups import cli from exasol.nb_connector.cli.options import SCS_OPTIONS from exasol.nb_connector.cli.param_wrappers import add_params from exasol.nb_connector.cli.processing import processing +from exasol.nb_connector.cli.processing.option_set import ScsCliError @cli.command() @@ -25,5 +27,8 @@ def check(scs_file: Path, connect: bool): Optionally also verify if a connection to the configured Exasol database instance is successful. """ - result = processing.check_scs(scs_file, connect) - sys.exit(result) + try: + processing.check_scs(scs_file, connect) + except ScsCliError as ex: + report.error(ex) + sys.exit(1) diff --git a/exasol/nb_connector/cli/commands/configure.py b/exasol/nb_connector/cli/commands/configure.py index 7bcca920..8be4b663 100644 --- a/exasol/nb_connector/cli/commands/configure.py +++ b/exasol/nb_connector/cli/commands/configure.py @@ -28,13 +28,12 @@ def configure_onprem(scs_file: Path, **kwargs): """ Configure connection to an Exasol on-premise instance. """ - result = processing.save( + processing.save( scs_file, StorageBackend.onprem, use_itde=False, values=kwargs, ) - sys.exit(result) @configure.command("saas") @@ -46,13 +45,12 @@ def configure_saas(scs_file: Path, **kwargs): Configuring one of the parameters --saas-database-id and --saas-database-name is sufficient. """ - result = processing.save( + processing.save( scs_file, StorageBackend.saas, use_itde=False, values=kwargs, ) - sys.exit(result) @configure.command("docker-db") @@ -61,10 +59,9 @@ def configure_docker_db(scs_file: Path, **kwargs): """ Configure connection to an Exasol Docker instance. """ - result = processing.save( + processing.save( scs_file, StorageBackend.onprem, use_itde=True, values=kwargs, ) - sys.exit(result) diff --git a/exasol/nb_connector/cli/commands/show.py b/exasol/nb_connector/cli/commands/show.py index 6bfcd442..86fb4f92 100644 --- a/exasol/nb_connector/cli/commands/show.py +++ b/exasol/nb_connector/cli/commands/show.py @@ -5,6 +5,8 @@ from exasol.nb_connector.cli.options import SCS_OPTIONS from exasol.nb_connector.cli.param_wrappers import add_params from exasol.nb_connector.cli.processing import processing +from exasol.nb_connector.cli.processing.option_set import ScsCliError +from exasol.nb_connector.cli import reporting as report @cli.command() @@ -14,5 +16,8 @@ def show(scs_file: Path): Show the configuration currently saved to the Secure Configuration Storage. """ - result = processing.show_scs_content(scs_file) - sys.exit(result) + try: + processing.show_scs_content(scs_file) + except ScsCliError as ex: + report.error(ex) + sys.exit(1) diff --git a/exasol/nb_connector/cli/processing/option_set.py b/exasol/nb_connector/cli/processing/option_set.py index 83487e42..159e1080 100644 --- a/exasol/nb_connector/cli/processing/option_set.py +++ b/exasol/nb_connector/cli/processing/option_set.py @@ -90,7 +90,7 @@ def set_dynamic_defaults(self, values: dict[str, Any]) -> dict[str, Any]: return values - def check(self) -> bool: + def check(self): """ Check if the content of the SCS is complete wrt. the selected backend as the required options depend on the selected backend. @@ -104,12 +104,11 @@ def check(self) -> bool: "Configuration is complete for an " f"Exasol {config.backend_name} instance." ) - return True + return formatted = ", ".join(missing) n = len(missing) prefix = "1 option is" if n == 1 else f"{n} options are" - report.error(f"{prefix} not yet configured: {formatted}.") - return False + raise ScsCliError(f"{prefix} not yet configured: {formatted}.") def get_scs_master_password(): @@ -128,7 +127,7 @@ def get_scs(scs_file: Path) -> Secrets: return Secrets(scs_file, scs_password) -def get_option_set(scs_file: Path) -> OptionSet | None: +def get_option_set(scs_file: Path) -> OptionSet: """ Return an instance of an OptionSet if the SCS contains a proper backend selection. Otherwise report an error and return None. @@ -144,6 +143,7 @@ def get_option_set(scs_file: Path) -> OptionSet | None: scs = get_scs(scs_file) config = BackendSelector(scs) if not config.knows_backend: - report.error(f"SCS {scs_file} does not contain any backend.") - return None + raise ScsCliError(f"SCS {scs_file} does not contain any backend.") + # report.error(f"SCS {scs_file} does not contain any backend.") + # return None return OptionSet(scs, config.backend, config.use_itde) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index d90f98cf..96333a39 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -20,6 +20,7 @@ SELECT_BACKEND_OPTION, USE_ITDE_OPTION, OptionSet, + ScsCliError, get_option_set, get_scs, ) @@ -35,7 +36,7 @@ def save( backend: StorageBackend, use_itde: bool, values: dict[str, Any], -) -> int: +): """ Save the provided values to SCS using the keys inferred from backend and use_itde. @@ -63,25 +64,22 @@ def save( continue if secret := option.get_secret(interactive=bool(value)): scs.save(option.scs_key, secret) - return 0 -def verify_connection(scs: Secrets) -> int: +def verify_connection(scs: Secrets) -> None: if BackendSelector(scs).use_itde: # Question: Is it OK, to let bring_itde_up modify the SCS content, here? False and bring_itde_up(scs) report.warning(f"Bring up ITDE currently disabled") - return 1 + return try: open_pyexasol_connection(scs).execute("SELECT 1 FROM DUAL").fetchone() except Exception as ex: - report.error(f"Failed to connect to the configured database {ex}") - return 1 + raise ScsCliError(f"Failed to connect to the configured database {ex}") report.success("Connection to the configured database instance was successful.") - return 0 -def check_scs(scs_file: Path, connect: bool) -> int: +def check_scs(scs_file: Path, connect: bool) -> None: """ Check the SCS content for completeness. Infer the required keys from backend and use_itde if these are contained in the SCS already. @@ -90,22 +88,19 @@ def check_scs(scs_file: Path, connect: bool) -> int: configured Exasol database instance is successful. """ options = get_option_set(scs_file) - if not options or not options.check(): - return 1 - return 0 if not connect else verify_connection(options.scs) + options.check() + if connect: + verify_connection(options.scs) -def show_scs_content(scs_file: Path) -> int: +def show_scs_content(scs_file: Path) -> None: """ If the SCS contains a proper backend selection, then show the SCS content for this context. """ - option_set = get_option_set(scs_file) - if not option_set: - return 1 - for o in option_set.options: - value = o.scs_key and o.displayed_value(option_set.scs) + oset = get_option_set(scs_file) + for o in oset.options: + value = o.scs_key and o.displayed_value(oset.scs) if value is not None: value = value or '""' click.echo(f"{o.cli_option()}: {value}") - return 0 diff --git a/exasol/nb_connector/cli/reporting.py b/exasol/nb_connector/cli/reporting.py index 58fe86c5..8a3a6cce 100644 --- a/exasol/nb_connector/cli/reporting.py +++ b/exasol/nb_connector/cli/reporting.py @@ -9,7 +9,7 @@ def success(text: str): click.echo(click.style(text, fg="green")) -def error(text: str): +def error(text: str | Exception): click.echo(click.style(f"Error: {text}", fg="bright_red")) diff --git a/test/unit/cli/test_option_set.py b/test/unit/cli/test_option_set.py index 7592293e..141ffb83 100644 --- a/test/unit/cli/test_option_set.py +++ b/test/unit/cli/test_option_set.py @@ -84,10 +84,8 @@ def test_valid_backend_configuration(scenario): @pytest.mark.parametrize("backend", [StorageBackend.onprem, None]) def test_option_set_backend_unknown(backend, capsys, scs_patcher): scs_patcher.patch(backend) - actual = get_option_set(Path("/fictional/scs")) - assert actual is None - message = "Error: SCS /fictional/scs does not contain any backend." - assert message in capsys.readouterr().out + with pytest.raises(ScsCliError, match="SCS .* does not contain any backend"): + get_option_set(Path("/fictional/scs")) @pytest.mark.parametrize( @@ -147,12 +145,13 @@ def test_check_failure(scenario, scs_patcher, capsys): o.cli_option(full=True) for o in scenario.params if o.scs_key and o.scs_required ] scs_patcher.patch(scenario.backend, scenario.use_itde) - testee = get_option_set(Path("/fictional/scs")) - assert not testee.check() - assert capsys.readouterr().out == ( - f"Error: {len(options)} options are not" - f" yet configured: {', '.join(options)}.\n" + expected = ( + f"{len(options)} options are not" + f" yet configured: {', '.join(options)}" ) + testee = get_option_set(Path("/fictional/scs")) + with pytest.raises(ScsCliError, match=expected): + testee.check() @pytest.mark.parametrize("scenario", TEST_SCENARIOS) @@ -161,8 +160,8 @@ def test_check_success(scenario, scs_patcher, capsys): scs_mock = scs_patcher.patch(scenario.backend, scenario.use_itde) for o in options: scs_mock.save(o.scs_key, "value") - testee = get_option_set(Path("/fictional/scs")) - assert testee.check() - assert capsys.readouterr().out == ( - f"Configuration is complete for an Exasol {scenario.name} instance.\n" + get_option_set(Path("/fictional/scs")).check() + assert ( + f"Configuration is complete for an Exasol {scenario.name} instance" + in capsys.readouterr().out ) diff --git a/test/unit/cli/test_processing_save.py b/test/unit/cli/test_processing_save.py index 5ffe8cf4..ec3b0e8d 100644 --- a/test/unit/cli/test_processing_save.py +++ b/test/unit/cli/test_processing_save.py @@ -18,13 +18,12 @@ def scs_patcher(monkeypatch): def test_save_overwrite_with_warning(scs_patcher, capsys): scs_mock = scs_patcher.patch(StorageBackend.saas, use_itde=False) - actual = processing.save( + processing.save( scs_file=Path("/fictional/scs"), backend=StorageBackend.onprem, use_itde=True, values={}, ) - assert actual == 0 assert get_backend(scs_mock) == StorageBackend.onprem assert scs_mock.get(CKey.use_itde) == "True" assert "Warning: Overwriting" in capsys.readouterr().out From fe55bd603cb20ef5f343467c0e2479519fab0a6f Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 1 Oct 2025 17:01:10 +0200 Subject: [PATCH 03/13] project:fix --- exasol/nb_connector/cli/commands/show.py | 2 +- test/unit/cli/test_cli.py | 2 +- test/unit/cli/test_option_set.py | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/exasol/nb_connector/cli/commands/show.py b/exasol/nb_connector/cli/commands/show.py index 86fb4f92..a0e17ff4 100644 --- a/exasol/nb_connector/cli/commands/show.py +++ b/exasol/nb_connector/cli/commands/show.py @@ -1,12 +1,12 @@ import sys from pathlib import Path +from exasol.nb_connector.cli import reporting as report from exasol.nb_connector.cli.groups import cli from exasol.nb_connector.cli.options import SCS_OPTIONS from exasol.nb_connector.cli.param_wrappers import add_params from exasol.nb_connector.cli.processing import processing from exasol.nb_connector.cli.processing.option_set import ScsCliError -from exasol.nb_connector.cli import reporting as report @cli.command() diff --git a/test/unit/cli/test_cli.py b/test/unit/cli/test_cli.py index 9a8f607c..7d47d8de 100644 --- a/test/unit/cli/test_cli.py +++ b/test/unit/cli/test_cli.py @@ -1,7 +1,7 @@ import getpass import itertools -from typing import Iterator import os +from collections.abc import Iterator from inspect import cleandoc from pathlib import Path from test.utils.integration_test_utils import sample_db_file diff --git a/test/unit/cli/test_option_set.py b/test/unit/cli/test_option_set.py index 141ffb83..b4a0cd59 100644 --- a/test/unit/cli/test_option_set.py +++ b/test/unit/cli/test_option_set.py @@ -146,8 +146,7 @@ def test_check_failure(scenario, scs_patcher, capsys): ] scs_patcher.patch(scenario.backend, scenario.use_itde) expected = ( - f"{len(options)} options are not" - f" yet configured: {', '.join(options)}" + f"{len(options)} options are not" f" yet configured: {', '.join(options)}" ) testee = get_option_set(Path("/fictional/scs")) with pytest.raises(ScsCliError, match=expected): @@ -163,5 +162,5 @@ def test_check_success(scenario, scs_patcher, capsys): get_option_set(Path("/fictional/scs")).check() assert ( f"Configuration is complete for an Exasol {scenario.name} instance" - in capsys.readouterr().out + in capsys.readouterr().out ) From ab87d1879b29254c1b656f1cf1e1284a1d4bf892 Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 1 Oct 2025 17:02:15 +0200 Subject: [PATCH 04/13] formatted --- test/unit/cli/test_option_set.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/cli/test_option_set.py b/test/unit/cli/test_option_set.py index b4a0cd59..9fd4e727 100644 --- a/test/unit/cli/test_option_set.py +++ b/test/unit/cli/test_option_set.py @@ -145,9 +145,7 @@ def test_check_failure(scenario, scs_patcher, capsys): o.cli_option(full=True) for o in scenario.params if o.scs_key and o.scs_required ] scs_patcher.patch(scenario.backend, scenario.use_itde) - expected = ( - f"{len(options)} options are not" f" yet configured: {', '.join(options)}" - ) + expected = f"{len(options)} options are not yet configured: {', '.join(options)}" testee = get_option_set(Path("/fictional/scs")) with pytest.raises(ScsCliError, match=expected): testee.check() From ffe4373f08e172d31b1af5abfb37bccc119bb10e Mon Sep 17 00:00:00 2001 From: ckunki Date: Thu, 2 Oct 2025 12:52:46 +0200 Subject: [PATCH 05/13] Removed dead code --- exasol/nb_connector/cli/processing/option_set.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/exasol/nb_connector/cli/processing/option_set.py b/exasol/nb_connector/cli/processing/option_set.py index 159e1080..05ed7704 100644 --- a/exasol/nb_connector/cli/processing/option_set.py +++ b/exasol/nb_connector/cli/processing/option_set.py @@ -144,6 +144,4 @@ def get_option_set(scs_file: Path) -> OptionSet: config = BackendSelector(scs) if not config.knows_backend: raise ScsCliError(f"SCS {scs_file} does not contain any backend.") - # report.error(f"SCS {scs_file} does not contain any backend.") - # return None return OptionSet(scs, config.backend, config.use_itde) From 3593c449fd18a9e52f515b63a82b90ab098c3f13 Mon Sep 17 00:00:00 2001 From: ckunki Date: Thu, 2 Oct 2025 14:50:45 +0200 Subject: [PATCH 06/13] Fixed review findings --- exasol/nb_connector/cli/processing/option_set.py | 2 +- exasol/nb_connector/cli/processing/processing.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/exasol/nb_connector/cli/processing/option_set.py b/exasol/nb_connector/cli/processing/option_set.py index 05ed7704..4dee1ce8 100644 --- a/exasol/nb_connector/cli/processing/option_set.py +++ b/exasol/nb_connector/cli/processing/option_set.py @@ -130,7 +130,7 @@ def get_scs(scs_file: Path) -> Secrets: def get_option_set(scs_file: Path) -> OptionSet: """ Return an instance of an OptionSet if the SCS contains a proper - backend selection. Otherwise report an error and return None. + backend selection. Otherwise raise an ScsCliError. This function is designed to be called only once in the CLI application. Otherwise it will always ask for the SCS master password and potentially diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index 96333a39..42109b84 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -67,6 +67,10 @@ def save( def verify_connection(scs: Secrets) -> None: + """ + Verify if successful connection to the configured backend is possible. + Raise an ScsCliError otherwise. + """ if BackendSelector(scs).use_itde: # Question: Is it OK, to let bring_itde_up modify the SCS content, here? False and bring_itde_up(scs) @@ -86,6 +90,14 @@ def check_scs(scs_file: Path, connect: bool) -> None: If parameter `connect` is True then also verify if a connection to the configured Exasol database instance is successful. + + The function raises an ScsCliError in any of the following cases: + + * The SCS does not select any backend. + + * The options are incomplete for configuring access to the selected backend. + + * Connecting to the configured backend was requested but failed. """ options = get_option_set(scs_file) options.check() From 867bf70a2e95516ffe317832ebcec8fbc57ab643 Mon Sep 17 00:00:00 2001 From: Christoph Kuhnke Date: Tue, 7 Oct 2025 08:35:03 +0200 Subject: [PATCH 07/13] Apply suggestions from code review Co-authored-by: Torsten Kilias --- exasol/nb_connector/cli/processing/option_set.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exasol/nb_connector/cli/processing/option_set.py b/exasol/nb_connector/cli/processing/option_set.py index 4dee1ce8..ead0bbb2 100644 --- a/exasol/nb_connector/cli/processing/option_set.py +++ b/exasol/nb_connector/cli/processing/option_set.py @@ -130,7 +130,7 @@ def get_scs(scs_file: Path) -> Secrets: def get_option_set(scs_file: Path) -> OptionSet: """ Return an instance of an OptionSet if the SCS contains a proper - backend selection. Otherwise raise an ScsCliError. + backend selection. Otherwise raise an ScsCliError. This function is designed to be called only once in the CLI application. Otherwise it will always ask for the SCS master password and potentially From 65a1ec8dd1422d610a5ee7e0d5e8c2a0e6342325 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 08:35:53 +0200 Subject: [PATCH 08/13] Cleanup regarding verify connection for Docker DB --- exasol/nb_connector/cli/processing/processing.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index 42109b84..cb188542 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -7,7 +7,6 @@ import click -from exasol.nb_connector.ai_lab_config import AILabConfig as CKey from exasol.nb_connector.ai_lab_config import StorageBackend from exasol.nb_connector.cli import reporting as report from exasol.nb_connector.cli.param_wrappers import ( @@ -25,7 +24,6 @@ get_scs, ) from exasol.nb_connector.connections import open_pyexasol_connection -from exasol.nb_connector.itde_manager import bring_itde_up from exasol.nb_connector.secret_store import Secrets LOG = logging.getLogger(__name__) @@ -70,11 +68,17 @@ def verify_connection(scs: Secrets) -> None: """ Verify if successful connection to the configured backend is possible. Raise an ScsCliError otherwise. + + For Docker-DB (ITDE) the connection is not verified currently, as + + * Startup of ITDE takes quite some time which is considered inconvienient + for the user + + * Connect does not verify parameters actually configured by the user but + rather parameters selected by ITDE """ if BackendSelector(scs).use_itde: - # Question: Is it OK, to let bring_itde_up modify the SCS content, here? - False and bring_itde_up(scs) - report.warning(f"Bring up ITDE currently disabled") + report.warning(f"Verification of connection with ITDE is not implemented, yet.") return try: open_pyexasol_connection(scs).execute("SELECT 1 FROM DUAL").fetchone() From 1665a3de434a90586fc954bd7380ed82246f9e5a Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 08:37:36 +0200 Subject: [PATCH 09/13] renamed oset to option_set --- exasol/nb_connector/cli/processing/processing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index cb188542..d3e796e5 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -114,9 +114,9 @@ def show_scs_content(scs_file: Path) -> None: If the SCS contains a proper backend selection, then show the SCS content for this context. """ - oset = get_option_set(scs_file) - for o in oset.options: - value = o.scs_key and o.displayed_value(oset.scs) + option_set = get_option_set(scs_file) + for o in option_set.options: + value = o.scs_key and o.displayed_value(option_set.scs) if value is not None: value = value or '""' click.echo(f"{o.cli_option()}: {value}") From ed7c728a6da552a0f570e870092fbc142f1f4707 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 10:04:43 +0200 Subject: [PATCH 10/13] Added unit test for check --connect --- test/unit/cli/conftest.py | 13 +++++++++++++ test/unit/cli/test_cli.py | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 test/unit/cli/conftest.py diff --git a/test/unit/cli/conftest.py b/test/unit/cli/conftest.py new file mode 100644 index 00000000..3f08ec51 --- /dev/null +++ b/test/unit/cli/conftest.py @@ -0,0 +1,13 @@ +from unittest.mock import create_autospec + +import pyexasol +import pytest + +from exasol.nb_connector.cli.processing import processing + + +@pytest.fixture +def pyexasol_connection_mock(monkeypatch): + mock = create_autospec(pyexasol.ExaConnection) + monkeypatch.setattr(processing, "open_pyexasol_connection", mock) + return mock diff --git a/test/unit/cli/test_cli.py b/test/unit/cli/test_cli.py index 7d47d8de..dcb2f88f 100644 --- a/test/unit/cli/test_cli.py +++ b/test/unit/cli/test_cli.py @@ -149,7 +149,13 @@ def test_configure(backend, expected, scs_with_env): ], ) def test_round_trip( - command, kwargs, env_opts, expected_show, monkeypatch, scs_with_env + command, + kwargs, + env_opts, + expected_show, + monkeypatch, + scs_with_env, + pyexasol_connection_mock, ): def cmd_args(): yield from [command, scs_file, "--db-schema", "SSS"] @@ -162,7 +168,16 @@ def cmd_args(): monkeypatch.setitem(os.environ, env_var, f"secret {i+1}") result = CliRunner().invoke(commands.configure, cmd_args()) assert result.exit_code == 0 - result = CliRunner().invoke(commands.check, [scs_file]) + + result = CliRunner().invoke(commands.check, [scs_file, "--connect"]) assert_success(result, f"Configuration is complete") + if command == "docker-db": + assert not pyexasol_connection_mock.called + assert ( + "Warning: Verification of connection with ITDE is not implemented, yet." + in result.output + ) + else: + assert pyexasol_connection_mock.called result = CliRunner().invoke(commands.show, [scs_file]) assert cleandoc(expected_show) in result.output From ac6fc17e554b53a1b17d6fa53eb3efe0ae08536d Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 10:58:00 +0200 Subject: [PATCH 11/13] Prepared verifying BucketFS connection --- .../nb_connector/cli/processing/processing.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index d3e796e5..2e3be147 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -68,18 +68,7 @@ def verify_connection(scs: Secrets) -> None: """ Verify if successful connection to the configured backend is possible. Raise an ScsCliError otherwise. - - For Docker-DB (ITDE) the connection is not verified currently, as - - * Startup of ITDE takes quite some time which is considered inconvienient - for the user - - * Connect does not verify parameters actually configured by the user but - rather parameters selected by ITDE """ - if BackendSelector(scs).use_itde: - report.warning(f"Verification of connection with ITDE is not implemented, yet.") - return try: open_pyexasol_connection(scs).execute("SELECT 1 FROM DUAL").fetchone() except Exception as ex: @@ -102,11 +91,24 @@ def check_scs(scs_file: Path, connect: bool) -> None: * The options are incomplete for configuring access to the selected backend. * Connecting to the configured backend was requested but failed. + + For Docker-DB (ITDE) the connection is not verified currently, as + + * Startup of ITDE takes quite some time which is considered inconvienient + for the user + + * Connect does not verify parameters actually configured by the user but + rather parameters selected by ITDE """ options = get_option_set(scs_file) options.check() - if connect: - verify_connection(options.scs) + if not connect: + return + scs = options.scs + if BackendSelector(scs).use_itde: + report.warning(f"Verification of connection with ITDE is not implemented, yet.") + return + verify_connection(scs) def show_scs_content(scs_file: Path) -> None: From 496492ac32b00b78560f9e233652297132cd39b2 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 12:54:39 +0200 Subject: [PATCH 12/13] Fixed exception raised in case of failure to connect --- exasol/nb_connector/cli/processing/processing.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index 2e3be147..835ac731 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +import traceback from enum import Enum from pathlib import Path from typing import Any @@ -72,7 +73,10 @@ def verify_connection(scs: Secrets) -> None: try: open_pyexasol_connection(scs).execute("SELECT 1 FROM DUAL").fetchone() except Exception as ex: - raise ScsCliError(f"Failed to connect to the configured database {ex}") + stacktrace = traceback.format_exc() + raise ScsCliError( + f"Failed to connect to the configured database {stacktrace}" + ) from ex report.success("Connection to the configured database instance was successful.") From 22f4f99fcfc14d125b6e90f0667526632e223c97 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 7 Oct 2025 15:47:50 +0200 Subject: [PATCH 13/13] Renamed variable "o" to "option" --- exasol/nb_connector/cli/processing/processing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exasol/nb_connector/cli/processing/processing.py b/exasol/nb_connector/cli/processing/processing.py index 835ac731..3b97cb12 100644 --- a/exasol/nb_connector/cli/processing/processing.py +++ b/exasol/nb_connector/cli/processing/processing.py @@ -121,8 +121,8 @@ def show_scs_content(scs_file: Path) -> None: content for this context. """ option_set = get_option_set(scs_file) - for o in option_set.options: - value = o.scs_key and o.displayed_value(option_set.scs) + for option in option_set.options: + value = option.scs_key and option.displayed_value(option_set.scs) if value is not None: value = value or '""' - click.echo(f"{o.cli_option()}: {value}") + click.echo(f"{option.cli_option()}: {value}")