diff --git a/src/databricks/labs/lakebridge/cli.py b/src/databricks/labs/lakebridge/cli.py index 9f32ecdbb6..05560bd1f6 100644 --- a/src/databricks/labs/lakebridge/cli.py +++ b/src/databricks/labs/lakebridge/cli.py @@ -225,12 +225,21 @@ class _TranspileConfigChecker: """The source dialect provided on the command-line, if any.""" _transpiler_repository: TranspilerRepository """The repository where available transpilers are installed.""" + _is_interactive: bool + """Whether the CLI is running interactively (stdin is a TTY). When False, prompting is disabled.""" + _TRANSPILER_OPTION_CLI_ARGS: dict[str, str] = { + "overrides-file": "--overrides-file", + "target-tech": "--target-technology", + } + """Mapping from transpiler option flag names to their corresponding CLI argument names.""" def __init__( self, config: TranspileConfig | None, prompts: Prompts, transpiler_repository: TranspilerRepository, + *, + is_interactive: bool | None = None, ) -> None: if config is None: logger.debug("No workspace transpile configuration, starting from defaults.") @@ -239,6 +248,16 @@ def __init__( self._prompts = prompts self._transpiler_repository = transpiler_repository self._source_dialect_override = None + self._is_interactive = sys.stdin.isatty() if is_interactive is None else is_interactive + + def _require_interactive(self, msg: str) -> None: + """Raise a validation error if the CLI is not running interactively. + + Call this before any prompt to ensure a clear error is raised instead of hanging when stdin is not a TTY + (e.g. when run non-interactively via the Databricks CLI). + """ + if not self._is_interactive: + raise_validation_exception(msg) @staticmethod def _validate_transpiler_config_path(transpiler_config_path: str, msg: str) -> None: @@ -317,6 +336,9 @@ def _prompt_input_source(self) -> None: def _check_input_source(self) -> None: config_input_source = self._config.input_source if config_input_source is None: + self._require_interactive( + "Missing required value for '--input-source': use '--input-source' to specify the SQL file or directory to convert." + ) self._prompt_input_source() else: self._validate_input_source( @@ -348,6 +370,9 @@ def _prompt_output_folder(self) -> None: def _check_output_folder(self) -> None: config_output_folder = self._config.output_folder if config_output_folder is None: + self._require_interactive( + "Missing required value for '--output-folder': use '--output-folder' to specify the output directory." + ) self._prompt_output_folder() else: self._validate_output_folder( @@ -426,6 +451,9 @@ def _configure_transpiler_config_path(self, source_dialect: str) -> TranspileEng logger.debug( f"Multiple transpilers available for dialect {source_dialect!r}: {compatible_transpilers!r}" ) + self._require_interactive( + f"Multiple transpilers are available for dialect {source_dialect!r}: use '--transpiler-config-path' to specify which one to use." + ) transpiler_name = self._prompts.choice("Select the transpiler:", list(compatible_transpilers)) transpiler_config_path = self._transpiler_repository.transpiler_config_path(transpiler_name) logger.info(f"Lakebridge will use the {transpiler_name} transpiler.") @@ -467,6 +495,9 @@ def _prompt_source_dialect(self) -> TranspileEngine: case _: # Multiple dialects available, prompt for which to use. logger.debug(f"Multiple source dialects available, choice required: {supported_dialects!r}") + self._require_interactive( + f"Multiple source dialects are available: use '--source-dialect' to specify one of: {', '.join(sorted(supported_dialects))}." + ) source_dialect = self._prompts.choice("Select the source dialect:", list(supported_dialects)) engine = self._configure_transpiler_config_path(source_dialect) assert engine is not None, "No transpiler engine available for a supported dialect; configuration is invalid." @@ -572,14 +603,17 @@ def _handle_missing_transpiler_option(self, option: LSPConfigOptionV1) -> JsonVa # - If the option has a default of that means that no value is required, no further action is required. # - Otherwise, a value is required: prompt for it. # - # TODO: When adding non-interactive support, the otherwise branch need to be modified: - # 1. If it can be provided by the command-line, fail and ask the user to provide it. - # 2. If it cannot be provided by the command-line, prompt for it if we are running interactively. - # 3. If we cannot prompt because we are not running interactively, use the default if there is one. - # 4. Fail: the only way to provide a value is via the config.yml, which can be set via 'install-transpile'. - if option.is_optional(): return None + cli_arg = self._TRANSPILER_OPTION_CLI_ARGS.get(option.flag) + if cli_arg is not None: + self._require_interactive( + f"Missing required transpiler option {option.flag!r}: use '{cli_arg}' to provide it." + ) + else: + self._require_interactive( + f"Missing required transpiler option {option.flag!r}: run 'install-transpile' to configure it." + ) return option.prompt_for_value(self._prompts) def check(self) -> tuple[TranspileConfig, TranspileEngine]: diff --git a/tests/unit/test_cli_transpile.py b/tests/unit/test_cli_transpile.py index d5fced186a..6073c70ca6 100644 --- a/tests/unit/test_cli_transpile.py +++ b/tests/unit/test_cli_transpile.py @@ -1,6 +1,7 @@ import dataclasses import json import re +import sys from collections.abc import Generator, Callable from unittest.mock import create_autospec, patch, ANY, MagicMock from pathlib import Path @@ -155,6 +156,7 @@ async def do_transpile(*args, **kwargs): with ( patch("databricks.labs.lakebridge.cli.do_transpile", new=do_transpile), patch("databricks.labs.lakebridge.cli.ApplicationContext", mock_app_context), + patch.object(sys.stdin, "isatty", return_value=True), ): mock_app_context.return_value.workspace_client = mock_workspace_client mock_app_context.return_value.prompts = prompts @@ -433,7 +435,7 @@ def test_transpile_prints_errors( prompts = MockPrompts({"Do you want to use the experimental.*": "no"}) ctx = ApplicationContext(ws=mock_workspace_client).replace(prompts=prompts) input_source = test_resources / "lsp_transpiler" / "unsupported_lca.sql" - with caplog.at_level("ERROR"): + with caplog.at_level("ERROR"), patch.object(sys.stdin, "isatty", return_value=True): cli.transpile( w=mock_workspace_client, transpiler_config_path=str(test_resources / "lsp_transpiler" / "lsp_config.yml"), @@ -508,6 +510,75 @@ def test_transpile_no_config_with_source_override( ) +@pytest.fixture +def mock_stdin_non_interactive(): + """Patch sys.stdin to simulate a non-interactive (non-TTY) environment.""" + with patch.object(sys.stdin, "isatty", return_value=False): + yield + + +def test_transpile_non_interactive_missing_input_source( + mock_cli_for_transpile, + transpiler_repository: TranspilerRepository, + mock_stdin_non_interactive, +) -> None: + """When stdin is not a TTY and --input-source is missing, raise a clear error instead of hanging.""" + ws, cfg, set_cfg, _ = mock_cli_for_transpile + set_cfg(dataclasses.replace(cfg, input_source=None)) + with pytest.raises(ValueError, match="Missing required value for '--input-source'"): + cli.transpile(w=ws, transpiler_repository=transpiler_repository) + + +def test_transpile_non_interactive_missing_output_folder( + mock_cli_for_transpile, + transpiler_repository: TranspilerRepository, + mock_stdin_non_interactive, +) -> None: + """When stdin is not a TTY and --output-folder is missing, raise a clear error instead of hanging.""" + ws, cfg, set_cfg, _ = mock_cli_for_transpile + set_cfg(dataclasses.replace(cfg, output_folder=None)) + with pytest.raises(ValueError, match="Missing required value for '--output-folder'"): + cli.transpile(w=ws, transpiler_repository=transpiler_repository) + + +def test_transpile_non_interactive_missing_source_dialect_with_multiple_dialects( + mock_cli_transpile_no_config, + transpiler_repository: TranspilerRepository, + empty_input_source: Path, + output_folder: Path, + mock_stdin_non_interactive, +) -> None: + """When stdin is not a TTY and --source-dialect is missing with multiple available dialects, raise a clear error.""" + ws, _, _ = mock_cli_transpile_no_config + # Provide input_source and output_folder so those checks pass; source_dialect remains unspecified. + with pytest.raises(ValueError, match="Multiple source dialects are available"): + cli.transpile( + w=ws, + input_source=str(empty_input_source), + output_folder=str(output_folder), + transpiler_repository=transpiler_repository, + ) + + +def test_transpile_non_interactive_missing_required_transpiler_option_with_cli_arg( + mock_cli_for_transpile, + transpiler_repository: TranspilerRepository, + mock_stdin_non_interactive, +) -> None: + """When stdin is not a TTY and a required transpiler option with a known CLI arg is missing, raise a clear error.""" + ws, cfg, set_cfg, _ = mock_cli_for_transpile + # Use informatica pc dialect which requires target-tech option + set_cfg( + dataclasses.replace( + cfg, + source_dialect="informatica pc", + transpiler_options={"overrides-file": None}, # target-tech is missing + ) + ) + with pytest.raises(ValueError, match=r"Missing required transpiler option 'target-tech'.*--target-technology"): + cli.transpile(w=ws, transpiler_repository=transpiler_repository) + + def test_describe_transpile(mock_cli_transpile_no_config, transpiler_repository: TranspilerRepository, capsys) -> None: """Verify that the 'describe-transpile' CLI command produces a JSON summary of installed transpilers.""" ws, _, _ = mock_cli_transpile_no_config