-
Notifications
You must be signed in to change notification settings - Fork 0
feat (cli-cicd integration): Enhance import command interface #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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.""" | ||
|
aviatco marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}'" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not specifically for items, right? if that's the case, let's rephrase the text to when item_type is not None
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I just enhance the error message, so we know which item_type is not supported. it is specific to item type |
||
|
|
||
| @staticmethod | ||
| def item_type_not_valid(item_type: str) -> str: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from user perspective, i am not aware of such flow.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so what do you suggests? |
||
| "$ 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)", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same.. this is transparent to users
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here @HasanAboShally |
||
| "$ 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 | ||
| import_parser.add_argument( | ||
| "path", nargs="+", type=str, help="Directory path (item name to import)" | ||
| "--config-file", | ||
| metavar="PATH", | ||
| type=str, | ||
| help="Path to local config file. When used, --env is required. Only used in CI/CD flow.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. local config file is overloaded term in the cli context. we should use some other distinct name
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cicd local config file?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. local import config file |
||
| ) | ||
|
|
||
| import_parser.add_argument( | ||
| "--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.", | ||
|
aviatco marked this conversation as resolved.
|
||
| ) | ||
|
|
||
| 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 | ||
| import_parser.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. 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)) | ||
|
aviatco marked this conversation as resolved.
|
||
| import_parser.usage = f"{utils_error_parser.get_usage_prog(import_parser)}" | ||
| import_parser.set_defaults(func=fs.import_command) | ||
|
|
||
|
|
||
| # Command for 'set' | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.