diff --git a/.changes/unreleased/added-20260210-123451.yaml b/.changes/unreleased/added-20260210-123451.yaml new file mode 100644 index 00000000..66794a21 --- /dev/null +++ b/.changes/unreleased/added-20260210-123451.yaml @@ -0,0 +1,6 @@ +kind: added +body: Update import command interface to support ci/cd flow +time: 2026-02-10T12:34:51.783593963Z +custom: + Author: aviatco + AuthorLink: https://github.com/aviatco diff --git a/src/fabric_cli/commands/fs/fab_fs.py b/src/fabric_cli/commands/fs/fab_fs.py index fb707302..9a78e158 100644 --- a/src/fabric_cli/commands/fs/fab_fs.py +++ b/src/fabric_cli/commands/fs/fab_fs.py @@ -140,7 +140,9 @@ def get_command(args: Namespace) -> None: @set_command_context() def import_command(args: Namespace) -> None: context = handle_context.get_command_context(args.path, raise_error=False) - context.check_command_support(Command.FS_IMPORT) + if hasattr(args, 'path') and args.path: + # In CICD flow - no path elements involved, skip command support check + context.check_command_support(Command.FS_IMPORT) fs_import.exec_command(args, context) diff --git a/src/fabric_cli/commands/fs/fab_fs_import.py b/src/fabric_cli/commands/fs/fab_fs_import.py index 67672c7d..dcf2beec 100644 --- a/src/fabric_cli/commands/fs/fab_fs_import.py +++ b/src/fabric_cli/commands/fs/fab_fs_import.py @@ -3,13 +3,22 @@ from argparse import Namespace -from fabric_cli.commands.fs.impor import fab_fs_import_item as import_item +from fabric_cli.commands.fs.impor import fab_fs_import_direct_api as import_direct_api +from fabric_cli.commands.fs.impor import fab_fs_import_cicd as import_cicd from fabric_cli.core.hiearchy.fab_hiearchy import FabricElement, Item from fabric_cli.utils import fab_util as utils def exec_command(args: Namespace, context: FabricElement) -> None: - args.input = utils.process_nargs(args.input) + if hasattr(args, 'validate_args'): + args.validate_args(args) - if isinstance(context, Item): - import_item.import_single_item(context, args) + # Direct API flow + if args.path and args.input: + args.input = utils.process_nargs(args.input) + if isinstance(context, Item): + import_direct_api.import_single_item(context, args) + + # CI/CD flow + elif args.config_file: + import_cicd.import_with_config_file(args) diff --git a/src/fabric_cli/commands/fs/impor/fab_fs_import_cicd.py b/src/fabric_cli/commands/fs/impor/fab_fs_import_cicd.py new file mode 100644 index 00000000..93ee73f2 --- /dev/null +++ b/src/fabric_cli/commands/fs/impor/fab_fs_import_cicd.py @@ -0,0 +1,4 @@ +from argparse import Namespace + +def import_with_config_file(args: Namespace) -> None: + """Import using config file and environment parameters - delegates to CICD library.""" \ No newline at end of file diff --git a/src/fabric_cli/commands/fs/impor/fab_fs_import_item.py b/src/fabric_cli/commands/fs/impor/fab_fs_import_direct_api.py similarity index 100% rename from src/fabric_cli/commands/fs/impor/fab_fs_import_item.py rename to src/fabric_cli/commands/fs/impor/fab_fs_import_direct_api.py diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index f81f9eb9..800d8108 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -282,6 +282,7 @@ ERROR_UNIVERSAL_SECURITY_DISABLED = "UniversalSecurityDisabled" ERROR_SPN_AUTH_MISSING = "ServicePrincipalAuthMissing" ERROR_JOB_FAILED = "JobFailed" +ERROR_IMPORT_VALIDATION = "ImportValidationError" # Exit codes EXIT_CODE_SUCCESS = 0 diff --git a/src/fabric_cli/core/hiearchy/fab_base_item.py b/src/fabric_cli/core/hiearchy/fab_base_item.py index 9de460fa..c4f4323c 100644 --- a/src/fabric_cli/core/hiearchy/fab_base_item.py +++ b/src/fabric_cli/core/hiearchy/fab_base_item.py @@ -100,7 +100,8 @@ def check_command_support(self, commmand: Command) -> bool: is_supported = self.item_type in cmd.get_supported_items(commmand) if is_unsupported: raise FabricCLIError( - ErrorMessages.Hierarchy.command_not_supported(commmand.value), + ErrorMessages.Hierarchy.command_not_supported( + commmand.value, str(self.item_type)), fab_constant.ERROR_UNSUPPORTED_COMMAND, ) if is_supported: @@ -109,7 +110,8 @@ def check_command_support(self, commmand: Command) -> bool: super().check_command_support(commmand) except FabricCLIError: raise FabricCLIError( - ErrorMessages.Hierarchy.command_not_supported(commmand.value), + ErrorMessages.Hierarchy.command_not_supported( + commmand.value, str(self.item_type)), fab_constant.ERROR_UNSUPPORTED_COMMAND, ) return True diff --git a/src/fabric_cli/core/hiearchy/fab_element.py b/src/fabric_cli/core/hiearchy/fab_element.py index b3f82aac..0ee2eb24 100644 --- a/src/fabric_cli/core/hiearchy/fab_element.py +++ b/src/fabric_cli/core/hiearchy/fab_element.py @@ -82,7 +82,8 @@ def check_command_support(self, commmand: Command) -> bool: is_supported = self.type in cmd.get_supported_elements(commmand) if not is_supported: raise FabricCLIError( - ErrorMessages.Hierarchy.command_not_supported(commmand.value), + ErrorMessages.Hierarchy.command_not_supported( + commmand.value, self.type.value), fab_constant.ERROR_UNSUPPORTED_COMMAND, ) return True diff --git a/src/fabric_cli/errors/common.py b/src/fabric_cli/errors/common.py index 801fafc0..e2c02530 100644 --- a/src/fabric_cli/errors/common.py +++ b/src/fabric_cli/errors/common.py @@ -244,6 +244,18 @@ def gateway_property_not_supported_for_type( property_name: str, gateway_type: str ) -> str: return f"Setting '{property_name}' is not supported for Gateway type '{gateway_type}'" + + @staticmethod + def import_required_param_missing(param_display_name: str, flow_name: str, flow_syntax: str) -> str: + return f"{param_display_name} is required for {flow_name} flow.\nCorrect syntax: {flow_syntax}" + + @staticmethod + def import_mixed_flows_error() -> str: + return "cannot mix Direct API flow parameters (path, -i, --format) with CICD flow parameters (--config-file, --env, -P)" + + @staticmethod + def import_no_flow_specified_error() -> str: + return "either -i/--input (with path) or --config-file (with --env) must be specified" @staticmethod def query_not_supported_for_set(query: str) -> str: diff --git a/src/fabric_cli/errors/hierarchy.py b/src/fabric_cli/errors/hierarchy.py index 41a95bb2..0252ee86 100644 --- a/src/fabric_cli/errors/hierarchy.py +++ b/src/fabric_cli/errors/hierarchy.py @@ -12,8 +12,8 @@ def invalid_type(name: str) -> str: return f"Invalid type '{name}'" @staticmethod - def command_not_supported(command: str) -> str: - return f"not supported for command '{command}'" + def command_not_supported(command: str, item_type: str) -> str: + return f"Command '{command}' is not supported for item type '{item_type}'" @staticmethod def item_type_not_valid(item_type: str) -> str: diff --git a/src/fabric_cli/parsers/fab_fs_parser.py b/src/fabric_cli/parsers/fab_fs_parser.py index 8d0273b0..1b6fe310 100644 --- a/src/fabric_cli/parsers/fab_fs_parser.py +++ b/src/fabric_cli/parsers/fab_fs_parser.py @@ -390,10 +390,14 @@ def register_get_parser(subparsers: _SubParsersAction) -> None: # Command for 'import' def register_import_parser(subparsers: _SubParsersAction) -> None: import_examples = [ - "# import a notebook from a local directory", + "# import a notebook from a local directory (Direct API call flow)", "$ import imp.Notebook -i /tmp/nb1.Notebook\n", - "# import a pipeline from a local directory", - "$ import pip.Notebook -i /tmp/pip1.DataPipeline -f", + "# import a pipeline from a local directory (Direct API call flow)", + "$ import pip.Notebook -i /tmp/pip1.DataPipeline -f\n", + "# import using config file and environment (CI/CD flow - no path needed)", + "$ import --config-file config.yml --env dev\n", + "# import with config file, environment, and CICD parameters (CI/CD flow)", + "$ import --config-file config.yml --env prod -P '[{\"param1\":\"value1\"}]' -f", ] import_parser = subparsers.add_parser( @@ -402,27 +406,62 @@ def register_import_parser(subparsers: _SubParsersAction) -> None: fab_examples=import_examples, fab_learnmore=["_"], ) + + # Create mutually exclusive groups for the two import flows + flow_group = import_parser.add_mutually_exclusive_group(required=True) + + #CI/CD flow parameters + flow_group.add_argument( + "--config-file", + metavar="PATH", + type=str, + help="Path to local config file. When used, --env is required. Only used in CI/CD flow.", + ) + import_parser.add_argument( - "path", nargs="+", type=str, help="Directory path (item name to import)" + "--env", + metavar="ENV_NAME", + type=str, + help="Environment name as defined in config file. Required when using --config-file. Only used in CI/CD flow.", ) + + import_parser.add_argument( + "-P", + "--params", + metavar="JSON", + type=str, + help="Optional parameters for CICD in JSON format (e.g., '[{\"p1\":\"v1\",\"p2\":\"v2\"}]'). Only used in CI/CD flow.", + ) + + # Direct API flow parameters + flow_group.add_argument( + "path", + nargs="*", # Changed from "+" to "*" to make it optional + type=str, + help="Directory path (item name to import). When used, -i/--input is required. Only used in Direct API call flow." + ) + import_parser.add_argument( "-i", "--input", nargs="+", - required=True, - help="Input path for import", + help="Input path for import. When used, path argument is required. Only used in Direct API call flow.", ) + import_parser.add_argument( "--format", metavar="", - help="Input format. Optional, supported for notebooks (.ipynb, .py)", + help="Input format for current flow only. Optional, supported for notebooks (.ipynb, .py). Only used in Direct API call flow.", ) + import_parser.add_argument( "-f", "--force", required=False, action="store_true", help="Force. Optional" ) + from fabric_cli.parsers.fab_parser_validators import validate_import_args + + import_parser.set_defaults(func=fs.import_command, validate_args=lambda args: validate_import_args(args)) import_parser.usage = f"{utils_error_parser.get_usage_prog(import_parser)}" - import_parser.set_defaults(func=fs.import_command) # Command for 'set' diff --git a/src/fabric_cli/parsers/fab_parser_validators.py b/src/fabric_cli/parsers/fab_parser_validators.py index 25895e53..8d4a8a57 100644 --- a/src/fabric_cli/parsers/fab_parser_validators.py +++ b/src/fabric_cli/parsers/fab_parser_validators.py @@ -2,6 +2,9 @@ # Licensed under the MIT License. import argparse +from fabric_cli.core import fab_constant +from fabric_cli.core.fab_exceptions import FabricCLIError +from fabric_cli.errors import ErrorMessages def validate_positive_int(value): @@ -12,4 +15,45 @@ def validate_positive_int(value): raise argparse.ArgumentTypeError(f"'{value}' must be a positive integer") return ivalue except ValueError: - raise argparse.ArgumentTypeError(f"'{value}' is not a valid integer") \ No newline at end of file + raise argparse.ArgumentTypeError(f"'{value}' is not a valid integer") + + +def validate_import_args(args): + """ + Validate import command arguments based on two distinct flows: + + Flow 1 (Direct API): path + input (--format optional) + Flow 2 (CICD): config-file + env (-P optional) + """ + + def _check_required_param(args, attr_name, param_display_name, flow_name, flow_syntax): + """Check if required parameter is provided for the specified flow.""" + param_value = getattr(args, attr_name, None) + if not param_value: + error_message = ErrorMessages.Common.import_required_param_missing(param_display_name, flow_name, flow_syntax) + raise FabricCLIError(error_message, fab_constant.ERROR_IMPORT_VALIDATION) + + is_direct_api_flow = args.path or args.input or args.format + is_cicd_flow = args.config_file or args.env or args.params + + if is_direct_api_flow and is_cicd_flow: + error_message = ErrorMessages.Common.import_mixed_flows_error() + raise FabricCLIError(error_message, fab_constant.ERROR_IMPORT_VALIDATION) + + if is_direct_api_flow: + flow_syntax = "fab import -i [--format ]" + _check_required_param(args, 'path', 'path', 'Direct API', flow_syntax) + _check_required_param(args, 'input', '-i/--input', + 'Direct API', flow_syntax) + + elif is_cicd_flow: + flow_syntax = "fab import --config-file --env [-P ]" + _check_required_param(args, 'config_file', + '--config-file', 'CICD', flow_syntax) + _check_required_param(args, 'env', '--env', 'CICD', flow_syntax) + + else: + error_message = ErrorMessages.Common.import_no_flow_specified_error() + raise FabricCLIError(error_message, fab_constant.ERROR_IMPORT_VALIDATION) + + return args diff --git a/tests/test_parsers/test_fab_parser_validators.py b/tests/test_parsers/test_fab_parser_validators.py index 1f4518bb..9b35ba9c 100644 --- a/tests/test_parsers/test_fab_parser_validators.py +++ b/tests/test_parsers/test_fab_parser_validators.py @@ -4,7 +4,11 @@ import argparse import pytest +from argparse import Namespace +from fabric_cli.core import fab_constant +from fabric_cli.core.fab_exceptions import FabricCLIError +from fabric_cli.errors import ErrorMessages from fabric_cli.parsers import fab_parser_validators @@ -50,3 +54,230 @@ def test_validate_positive_int_with_non_numeric_strings_raises_error(): def test_validate_positive_int_with_none_raises_error(): with pytest.raises(TypeError): fab_parser_validators.validate_positive_int(None) + + +# Import validation tests +def test_import_direct_api_flow_valid_args_success(): + """Test Direct API flow with valid arguments succeeds.""" + args = create_args( + path=["ws1.Workspace/nb1.Notebook"], + input=["/path/to/input"] + ) + + try: + validated_args = fab_parser_validators.validate_import_args(args) + assert validated_args == args + assert validated_args.path == ["ws1.Workspace/nb1.Notebook"] + assert validated_args.input == ["/path/to/input"] + except FabricCLIError: + pytest.fail( + "validate_import_args() should not have raised FabricCLIError for valid Direct API flow") + + +def test_import_direct_api_flow_with_format_success(): + """Test Direct API flow with optional format parameter succeeds.""" + args = create_args( + path=["ws1.Workspace/nb1.Notebook"], + input=["/path/to/input"], + format=".ipynb" + ) + + try: + validated_args = fab_parser_validators.validate_import_args(args) + assert validated_args == args + assert args.format == ".ipynb" + except FabricCLIError: + pytest.fail( + "validate_import_args() should not have raised FabricCLIError for valid Direct API flow with format") + + +def create_args(**kwargs): + """Helper function to create Namespace with all values set to None, then override specific values.""" + defaults = { + 'path': None, + 'input': None, + 'format': None, + 'config_file': None, + 'env': None, + 'params': None + } + defaults.update(kwargs) + return Namespace(**defaults) + + +@pytest.mark.parametrize("args,param_display_name", [ + (create_args(input=["/path/to/input"]), "path"), + (create_args(path=["ws1.Workspace/nb1.Notebook"]), "-i/--input") +]) +def test_import_direct_api_flow_missing_required_params_fails(args, param_display_name): + """Test Direct API flow fails when required parameters are missing.""" + flow_name = "Direct API" + flow_syntax = "fab import -i [--format ]" + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + expected_message = ErrorMessages.Common.import_required_param_missing( + param_display_name, flow_name, flow_syntax) + assert exc_info.value.message == expected_message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_cicd_flow_valid_args_success(): + """Test CICD flow with valid arguments succeeds.""" + args = create_args( + config_file="/path/to/config.yml", + env="production" + ) + + try: + validated_args = fab_parser_validators.validate_import_args(args) + assert validated_args == args + assert args.config_file == "/path/to/config.yml" + assert args.env == "production" + except FabricCLIError: + pytest.fail( + "validate_import_args() should not have raised FabricCLIError for valid CICD flow") + + +def test_import_cicd_flow_with_params_success(): + """Test CICD flow with optional params parameter succeeds.""" + args = create_args( + config_file="/path/to/config.yml", + env="production", + params='[{"param1": "value1"}]' + ) + + try: + validated_args = fab_parser_validators.validate_import_args(args) + assert validated_args == args + assert args.params == '[{"param1": "value1"}]' + except FabricCLIError: + pytest.fail( + "validate_import_args() should not have raised FabricCLIError for valid CICD flow with params") + + +@pytest.mark.parametrize("args,param_display_name", [ + (create_args(config_file="/path/to/config.yml"), "--env"), + (create_args(env="production"), "--config-file") +]) +def test_import_cicd_flow_missing_required_params_fails(args, param_display_name): + """Test CICD flow fails when required parameters are missing.""" + flow_name = "CICD" + flow_syntax = "fab import --config-file --env [-P ]" + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + expected_message = ErrorMessages.Common.import_required_param_missing( + param_display_name, flow_name, flow_syntax) + assert exc_info.value.message == expected_message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_no_flow_specified_fails(): + """Test that no flow specified results in error.""" + args = create_args() + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + expected_message = ErrorMessages.Common.import_no_flow_specified_error() + assert exc_info.value.message == expected_message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_mixed_flows_fails(): + """Test that mixing Direct API and CICD parameters fails.""" + args = create_args( + path=["ws1.Workspace/nb1.Notebook"], # Direct API + input=["/path/to/input"], # Direct API + config_file="/path/to/config.yml", # CICD + env="production" # CICD + ) + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + expected_message = ErrorMessages.Common.import_mixed_flows_error() + assert exc_info.value.message == expected_message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_error_messages_include_syntax_examples(): + """Test that error messages include correct syntax examples.""" + args = create_args(path=["ws1.Workspace/nb1.Notebook"]) + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "-i/--input is required for Direct API flow" in exc_info.value.message + assert "Correct syntax: fab import -i [--format ]" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_cicd_error_messages_include_syntax_examples(): + """Test that CICD flow error messages include correct syntax examples.""" + args = create_args(config_file="/path/to/config.yml") + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "--env is required for CICD flow" in exc_info.value.message + assert "Correct syntax: fab import --config-file --env [-P ]" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_path_only_triggers_direct_api_validation(): + """Test that providing only path triggers Direct API flow validation.""" + args = create_args(path=["ws1.Workspace/nb1.Notebook"]) + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "-i/--input is required for Direct API flow" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_input_only_triggers_direct_api_validation(): + """Test that providing only input triggers Direct API flow validation.""" + args = create_args(input=["/path/to/input"]) + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "path is required for Direct API flow" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_format_only_triggers_direct_api_validation(): + """Test that providing only format triggers Direct API flow validation.""" + args = create_args(format=".ipynb") + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "path is required for Direct API flow" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_env_only_triggers_cicd_validation(): + """Test that providing only env triggers CICD flow validation.""" + args = create_args(env="production") + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "--config-file is required for CICD flow" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION + + +def test_import_params_only_triggers_cicd_validation(): + """Test that providing only params triggers CICD flow validation.""" + args = create_args(params='[{"key": "value"}]') + + with pytest.raises(FabricCLIError) as exc_info: + fab_parser_validators.validate_import_args(args) + + assert "--config-file is required for CICD flow" in exc_info.value.message + assert exc_info.value.status_code == fab_constant.ERROR_IMPORT_VALIDATION