feat (cli-cicd integration): Enhance import command interface by aviatco · Pull Request #1 · aviatco/fabric-cli

aviatco · 2026-02-10T16:40:07Z

📥 Pull Request

✨ Description of new changes

This pull request introduces significant improvements to the import command interface in the CLI, enabling support for both Direct API and CI/CD flows with clear argument validation and user guidance. The changes add mutually exclusive argument groups for the two flows, enforce correct usage with custom validators, improve error messages, and update documentation and examples to reflect the new interface.

Import Command Interface and Validation:

The import command now supports two mutually exclusive flows: Direct API (using path and -i/--input) and CI/CD (using --config-file and --env). Argument parsing enforces that only one flow can be used at a time, and provides detailed error messages if parameters are missing or flows are mixed. (src/fabric_cli/parsers/fab_fs_parser.py [1] [2]; src/fabric_cli/parsers/fab_parser_validators.py [3]
A new validation function validate_import_args ensures that required parameters for each flow are provided and that the flows are not mixed. Custom error messages guide the user to correct syntax. (src/fabric_cli/parsers/fab_parser_validators.py [1]; src/fabric_cli/errors/common.py [2]
The import_command and its implementation have been updated to handle both flows, delegating to the correct import logic based on the arguments. (src/fabric_cli/commands/fs/fab_fs.py [1]; src/fabric_cli/commands/fs/fab_fs_import.py [2]

Error Handling and Messages:

Improved error messages for unsupported commands, missing parameters, and incorrect flow usage, including more descriptive context about item types and flows. (src/fabric_cli/errors/hierarchy.py [1]; src/fabric_cli/core/hiearchy/fab_base_item.py [2] [3]; src/fabric_cli/core/hiearchy/fab_element.py [4]; src/fabric_cli/errors/common.py [5]
Added a new error code for import validation errors. (src/fabric_cli/core/fab_constant.py src/fabric_cli/core/fab_constant.pyR285)

Documentation and Examples:

Updated CLI help and examples to clearly differentiate between Direct API and CI/CD flows, providing sample usage for both. (src/fabric_cli/parsers/fab_fs_parser.py src/fabric_cli/parsers/fab_fs_parser.pyL393-R400)

Codebase Cleanup:

Removed the obsolete fab_fs_import_item.py file, as its functionality is now split between flow-specific modules. (src/fabric_cli/commands/fs/impor/fab_fs_import_item.py src/fabric_cli/commands/fs/impor/fab_fs_import_item.pyL1-L126)

Changelog:

Added an unreleased changelog entry describing the update to the import command interface for CI/CD flow support. (.changes/unreleased/added-20260210-123451.yaml .changes/unreleased/added-20260210-123451.yamlR1-R6)

ayeshurun · 2026-02-11T08:16:08Z

src/fabric_cli/errors/common.py

+
+    @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}"


not sure it actually break into a new line.

it is - I test it.
but I see not the string is wrong - I wanted it without the "correct" - will remove it

ayeshurun · 2026-02-15T20:54:05Z

src/fabric_cli/commands/fs/impor/fab_fs_import_cicd.py

+from argparse import Namespace
+
+def import_with_config_file(args: Namespace) -> None:
+    """Import using config file and environment parameters - delegates to CICD library."""


why do we need specific file for it and can't it be in the same file of the regular process?

ayeshurun · 2026-02-15T20:59:31Z

src/fabric_cli/commands/fs/fab_fs.py

    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


what would be if i'm in some path and do import --config-file?

e.g.,

fab:ws.workspace/$ import --config-file <>

ayeshurun · 2026-02-15T21:00:48Z

src/fabric_cli/commands/fs/fab_fs_import.py

+    elif args.config_file:
+        import_cicd.import_with_config_file(args)


else? do we need want to return missing input? what happened?

ayeshurun · 2026-02-15T21:02:33Z

src/fabric_cli/errors/common.py

+
+    @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}"


can you give example of correct syntax?

nit: i'd phrase it differently like, Please use ... instead

ayeshurun · 2026-02-15T21:02:57Z

src/fabric_cli/errors/common.py

+
+    @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)"


nit: capital C

ayeshurun · 2026-02-15T21:04:25Z

src/fabric_cli/errors/common.py

+
+    @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)"


this message isn't clear to users. they don't know what direct api vs config. it should be something like

arguments mismatch. you cannot use combination from path/input with config-file/env/params

ayeshurun · 2026-02-15T21:04:46Z

src/fabric_cli/errors/common.py

+
+    @staticmethod
+    def import_no_flow_specified_error() -> str:
+        return "either -i/--input (with path) or --config-file (with --env) must be specified"


capital E

ayeshurun · 2026-02-15T21:05:22Z

src/fabric_cli/errors/common.py

+
+    @staticmethod
+    def import_no_flow_specified_error() -> str:
+        return "either -i/--input (with path) or --config-file (with --env) must be specified"


this text isn't clear. can we think of something else?

ayeshurun · 2026-02-15T21:06:11Z

src/fabric_cli/errors/hierarchy.py

-    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}'"


not specifically for items, right? if that's the case, let's rephrase the text to when item_type is not None

ayeshurun · 2026-02-15T21:06:42Z

src/fabric_cli/parsers/fab_fs_parser.py

 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)",


from user perspective, i am not aware of such flow.

ayeshurun · 2026-02-15T21:07:13Z

src/fabric_cli/parsers/fab_fs_parser.py

-        "$ 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)",


same.. this is transparent to users

ayeshurun · 2026-02-15T21:09:56Z

src/fabric_cli/parsers/fab_fs_parser.py

+        "--config-file",
+        metavar="PATH",
+        type=str,
+        help="Path to local config file. When used, --env is required. Only used in CI/CD flow.",


local config file is overloaded term in the cli context. we should use some other distinct name

ayeshurun · 2026-02-15T21:11:19Z

src/fabric_cli/parsers/fab_fs_parser.py

+        "--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.",


i think environment also is a bit overloaded. what do you think about --stage ? @shirasassoon ?

ayeshurun · 2026-02-15T21:12:22Z

src/fabric_cli/parsers/fab_fs_parser.py

        "--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.",


what does it mean 'current flow only'?

ayeshurun · 2026-02-15T21:15:38Z

src/fabric_cli/core/fab_constant.py

 ERROR_UNIVERSAL_SECURITY_DISABLED = "UniversalSecurityDisabled"
 ERROR_SPN_AUTH_MISSING = "ServicePrincipalAuthMissing"
 ERROR_JOB_FAILED = "JobFailed"
+ERROR_IMPORT_VALIDATION = "ImportValidationError"


InvalidImportArguments ?

ayeshurun · 2026-02-15T21:17:11Z

src/fabric_cli/parsers/fab_parser_validators.py

+        _check_required_param(args, 'path', 'path', 'Direct API', flow_syntax)
+        _check_required_param(args, 'input', '-i/--input',
+                              'Direct API', flow_syntax)


looks to me too hard coded. tomorrow we will change in the parser the text and we will forget this place. can we think on another approach?

ayeshurun · 2026-02-15T21:35:31Z

src/fabric_cli/parsers/fab_fs_parser.py

+    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))


i think it is not necessary, we use this only in one place, hence we can validate the import_args there

Aviat Cohen added 5 commits February 10, 2026 16:23

enhance import command to support cicd attribute

7d8b3c3

add changie

3f0eef9

fix tests

12b62cc

add import commad

15e6728

merge with main and fix conflicts

d2add84

aviatco marked this pull request as ready for review February 10, 2026 16:50

ayeshurun reviewed Feb 11, 2026

View reviewed changes

ayeshurun reviewed Feb 15, 2026

View reviewed changes

		elif args.config_file:
		import_cicd.import_with_config_file(args)

		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))

Conversation

aviatco commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📥 Pull Request

✨ Description of new changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aviatco commented Feb 10, 2026 •

edited

Loading