feat: support bulk-export command#261
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new bulk-export command to Fabric CLI (fab) to export all supported items from a workspace or folder in one operation, preserving folder structure and item bindings. It includes end-to-end command wiring (parser → command handler → API client), export utilities for writing bulk-export definition parts to storage, debug-log redaction improvements, and corresponding documentation + tests (including VCR recordings).
Changes:
- Implemented
bulk-exportcommand flow (parsing, command dispatch, workspace/folder execution paths, API call, and export-to-storage utilities). - Extended export utilities and HTTP debug logging to better support bulk export payload shapes and reduce noisy/large debug logs.
- Added docs pages/examples and comprehensive tests + recordings for the new command.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils/test_fab_cmd_bulk_export_utils.py | Unit tests for bulk-export utility helpers (payload creation, summary output, path stripping/export). |
| tests/test_core/test_fab_logger.py | Updates logger tests for new command-context-aware response logging/redaction. |
| tests/test_commands/test_bulk_export.py | New command-level tests for bulk-export behavior and output structure. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_workspace_without_recursive_fail.yaml | VCR recording for workspace bulk-export missing --recursive. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_workspace_folder_without_recursive_fail.yaml | VCR recording for folder bulk-export missing --recursive. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_output_path_not_empty_warning.yaml | VCR recording for warning behavior when output directory is non-empty. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_no_exportable_items_fail.yaml | VCR recording for “no exportable items” failure case. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_item_fail.yaml | VCR recording for invalid target type (item path). |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_empty_workspace_fail.yaml | VCR recording for empty workspace failure case. |
| tests/test_commands/recordings/test_commands/test_bulk_export/test_bulk_export_empty_folder_fail.yaml | VCR recording for empty folder failure case. |
| tests/test_commands/recordings/test_commands/test_bulk_export/class_setup.yaml | VCR class setup recording for bulk-export command tests. |
| tests/test_commands/commands_parser.py | Registers the new bulk-export parser into the command test harness. |
| src/fabric_cli/utils/fab_commands.py | Adds bulk-export to the CLI command index text. |
| src/fabric_cli/utils/fab_cmd_export_utils.py | Extends decode/export helpers to support definitionParts payload shape. |
| src/fabric_cli/utils/fab_cmd_bulk_export_utils.py | New bulk-export utility module (payload, export writing, summary printing, path validation). |
| src/fabric_cli/parsers/fab_fs_parser.py | Adds argparse registration for the bulk-export command. |
| src/fabric_cli/errors/bulk_export.py | Defines bulk-export-specific error messages. |
| src/fabric_cli/core/fab_parser_setup.py | Wires bulk-export parser into the global CLI parser setup. |
| src/fabric_cli/core/fab_logger.py | Adds command context to response logging + JSON response redaction logic. |
| src/fabric_cli/core/fab_constant.py | Adds COMMAND_FS_BULKEXPORT_DESCRIPTION. |
| src/fabric_cli/core/fab_config/command_support.yaml | Registers bulk-export support matrix (workspace/folder + supported item types). |
| src/fabric_cli/core/fab_commands.py | Adds FS_BULKEXPORT command enum value. |
| src/fabric_cli/commands/fs/fab_fs.py | Hooks bulk_export_command into the fs command module. |
| src/fabric_cli/commands/fs/fab_fs_bulk_export.py | New bulk-export command implementation and precondition checks. |
| src/fabric_cli/commands/fs/bulk_export/fab_fs_bulk_export_workspace.py | Workspace bulk-export execution and response filtering. |
| src/fabric_cli/commands/fs/bulk_export/fab_fs_bulk_export_folder.py | Folder bulk-export execution and recursive item collection. |
| src/fabric_cli/commands/fs/bulk_export/init.py | Package init for bulk-export submodule. |
| src/fabric_cli/client/fab_api_item.py | Adds bulk_export_definitions() API wrapper. |
| src/fabric_cli/client/fab_api_client.py | Passes command context into HTTP response debug logging. |
| mkdocs.yml | Adds bulk-export page to MkDocs navigation. |
| docs/examples/workspace_examples.md | Adds workspace bulk-export example section. |
| docs/examples/folder_examples.md | Adds folder bulk-export example section. |
| docs/commands/index.md | Adds bulk-export to the command index page. |
| docs/commands/fs/bulk_export.md | New bulk-export command documentation page. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/fabric_cli/utils/fab_cmd_export_utils.py:63
- The docstring still says this function exports from the
'parts'array, but the implementation now supports an arbitrary key viadefinition_parts(e.g.,definitionPartsfor bulk-export). Updating the docstring avoids misleading future callers.
"""
Export each 'payload' in the 'parts' array to a file named based on 'path' in 'parts'.
The 'payload' content will be saved as the file's content.
"""
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/fabric_cli/commands/fs/fab_fs_bulk_export.py:61
- The output directory validation rejects non-existent local paths (
not os.path.isdir(...)), which prevents creating a new output folder even though the code later callsos.makedirs(...). This makesbulk-export -o <new-dir>fail unnecessarily.
export_path = fab_storage.get_export_path(args.output)
if (export_path["type"] != "local") or (
export_path["type"] == "local" and not os.path.isdir(export_path["path"])
):
raise FabricCLIError(
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/fabric_cli/utils/fab_cmd_bulk_export_utils.py:155
- The path-traversal guard uses a plain string
startswith(...)check, which is case-sensitive and can behave incorrectly on Windows (drive-letter casing) and is generally less robust than a path-based containment check. Usingos.path.commonpathwithnormcasemakes the intent explicit and avoids false negatives/positives across platforms.
"""Validate that all definition part paths concatenated with the export path are under the export path to prevent path traversal issues."""
for part in item_def.get("definitionParts", []):
part_path = part.get("path", "").lstrip("/")
full_export_path = os.path.abspath(os.path.join(export_path, part_path))
if not full_export_path.startswith(os.path.abspath(export_path) + os.sep):
src/fabric_cli/utils/fab_cmd_bulk_export_utils.py:120
- The docstring example shows
from_pathincluding an item segment (.../n1.Notebook), butbulk-exportonly accepts workspace/folder targets and this helper is called with those targets. Updating the example to a folder target will prevent confusion about what prefixes are stripped.
"""Remove the parent prefix from each part's path so only the bulk-export target remains.
For example, if from_path is "myws.Workspace/f1.Folder/f2.Folder/n1.Notebook", the
workspace segment is skipped (not part of definitionParts paths) and the prefix
"/f1/f2/" is stripped from each definition part path (folder names without .Folder suffix).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 42 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/fabric_cli/utils/fab_cmd_bulk_export_utils.py:156
- Path traversal validation uses
full_export_path.startswith(os.path.abspath(export_path) + os.sep), which breaks for root export paths (e.g. export_path == "/" yields "//"), causing safe exports to be rejected. Usingos.path.commonpathavoids false negatives while still preventing traversal.
for part in item_def.get("definitionParts", []):
part_path = part.get("path", "").lstrip("/")
full_export_path = os.path.abspath(os.path.join(export_path, part_path))
if not full_export_path.startswith(os.path.abspath(export_path) + os.sep):
raise FabricCLIError(
src/fabric_cli/commands/fs/bulk_export/fab_fs_bulk_export_workspace.py:84
is_command_supported()ultimately callsItem.check_command_support(), which returns True or raisesFabricCLIError(it never returns False). The current logic has an unreachableelsebranch and also treats any workspace item missing fromitemDefinitionsIndexas an "unsupported item type", which can misreport supported items as skipped (e.g., items omitted due to permissions/API behavior). Summary should only classify items that are explicitly in the API response index.
try:
if bulk_export_utils.is_command_supported(ws_item):
supported_item_prefixes.add(root_path)
supported_items_list.append(ws_item)
else:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 42 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/fabric_cli/commands/fs/bulk_export/fab_fs_bulk_export_workspace.py:97
- This block treats any workspace item missing from
itemDefinitionsIndexas an “unsupported item type”. The bulk-export API can omit items for other reasons (for example insufficient permissions), so counting them as unsupported will produce an incorrect “Skipped … due to unsupported item types” summary.
if (
ws_item.id not in supported_item_ids
and ws_item.id not in unsupported_item_ids
):
unsupported_items_list.append(ws_item)
tests/test_core/test_fab_logger.py:134
FAB_DEBUG_ENABLEDis validated/configured as the strings "true"/"false" (seeFAB_CONFIG_KEYS_TO_VALID_VALUES), but these tests use "1"/"0". As a result, the "enabled" branch is never exercised and the tests won’t cover the JSON logging logic.
| for part in item_def.get("definitionParts", []): | ||
| part_path = part.get("path", "").lstrip("/") | ||
| full_export_path = os.path.abspath(os.path.join(export_path, part_path)) | ||
| if not full_export_path.startswith(os.path.abspath(export_path) + os.sep): |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 42 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
tests/test_commands/commands_parser.py:70
register_bulk_export_parseris included twice inparserHandlers. Registering the same subcommand twice will raise an argparse error for duplicate parser names (e.g.,bulk-export) and can break command parsing in tests.
register_start_parser,
register_set_parser,
register_open_parser,
register_rm_parser,
register_mkdir_parser,
register_jobs_parser,
register_bulk_export_parser,
]
src/fabric_cli/commands/fs/bulk_export/fab_fs_bulk_export_workspace.py:98
- Workspace bulk-export currently treats any workspace item that is not present in
itemDefinitionsIndexas an “unsupported item” (and also contains an unreachableelsebranch becauseis_command_supported()returns True or raises). Items can be missing from the response for reasons other than type support (e.g., API omitting items due to permissions), so counting them as “unsupported item types” makes the summary misleading and can inflateskipped.
try:
if bulk_export_utils.is_command_supported(ws_item):
supported_item_prefixes.add(root_path)
supported_items_list.append(ws_item)
else:
unsupported_items_list.append(ws_item)
except FabricCLIError:
unsupported_items_list.append(ws_item)
# find unsupported items that were not in the response index and add them to the unsupported_items_list
supported_item_ids: set[str] = {item.id for item in supported_items_list}
unsupported_item_ids: set[str] = {item.id for item in unsupported_items_list}
for ws_item in ws_items:
if (
ws_item.id not in supported_item_ids
and ws_item.id not in unsupported_item_ids
):
unsupported_items_list.append(ws_item)
| BulkExportErrors.empty_target(workspace.name), | ||
| ) | ||
|
|
||
| def test_bulk_export_invalid_output_path_fail( |
| folder = folder_factory() | ||
| _ = item_factory(ItemType.NOTEBOOK, path=folder.full_path) | ||
|
|
||
| # Execute command with --preserve_binding but without --recursive (which is required for workspace/folder) |
There was a problem hiding this comment.
what is --preserve_binding ?
is this comment relevant?
| _ = item_factory(ItemType.NOTEBOOK, path=folder1.full_path) | ||
| notebook2 = item_factory(ItemType.NOTEBOOK, path=folder2.full_path) | ||
|
|
||
| with patch("fabric_cli.utils.fab_ui.print_output_format") as mock_print_output: |
There was a problem hiding this comment.
why not use questionary_mock or other print fixture with have?
✨ Description of new changes
This pull request introduces a new
bulk-exportcommand to the CLI, enabling bulk export of all supported items from a workspace or folder while preserving folder structure and item bindings. The implementation includes API integration, argument validation, error handling, and comprehensive documentation and examples. The changes also update command support configuration and the command index.New Functionality:
bulk-exportcommand that allows users to export all supported items from a workspace or folder in a single operation, preserving folder structure and item bindings. This includes both CLI logic and integration with the backend API. [1] [2] [3] [4] [5]Documentation Updates:
bulk_export.mddetailing usage, parameters, supported item types, output structure, and examples for thebulk-exportcommand.bulk-export. [1] [2]Command Support and Configuration:
bulkexportcommand in the CLI command enumeration and in the command support YAML, specifying supported elements (workspace, folder) and item types. [1] [2]Internal Integration and Validation:
bulk-exportcommand into the main command handler and ensured argument validation, context checking, and user confirmation logic are in place. [1] [2] [3]Minor Improvements:
These changes collectively introduce and document the new
bulk-exportfeature, making it easier for users to export entire workspaces or folders with structure and bindings preserved.# 📥 Pull Request