Conversation
There was a problem hiding this comment.
Pull request overview
Removes the CLI’s hardcoded default "description" field from various create/import payloads so descriptions are only sent when intentionally provided, improving payload predictability.
Changes:
- Removed default
"description"stamping from item import payloads and environment import-create payloads. - Removed default
"description"stamping from multiplemkdirpayload builders (workspace, item, folder, connection, gateway). - Updated/added tests to stop expecting default descriptions and to assert description absence by default.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils/test_fab_cmd_mkdir_utils.py | Updates existing connection-payload tests and adds new assertions about description presence/absence. |
| tests/test_core/test_fab_hiearchy.py | Removes "Imported from fab" expectations and adds a regression test that import payloads don’t auto-stamp description. |
| src/fabric_cli/utils/fab_cmd_import_utils.py | Removes hardcoded "Imported from fab" from get_payload_for_item_type() payloads. |
| src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_workspace.py | Removes hardcoded "Created by fab" from workspace create payload. |
| src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py | Removes hardcoded "Created by fab" from item create payload. |
| src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py | Removes leftover commented default description from gateway payload block. |
| src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py | Removes hardcoded "Created by fab" from folder create payload. |
| src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py | Removes hardcoded "Created by fab" from connection create payload. |
| src/fabric_cli/commands/fs/impor/fab_fs_import_item.py | Removes hardcoded "Imported from fab" from environment item create payload. |
| utils_ui.print_grey(f"Creating a new Connection...") | ||
|
|
||
| # Base payload | ||
| payload = { | ||
| "description": "Created by fab", | ||
| "displayName": connection.short_name, | ||
| "connectivityType": connectivityType, | ||
| } |
There was a problem hiding this comment.
description is listed as an optional param for mkdir connection, but it is never copied into the request payload. After removing the hardcoded default description, any user-supplied description in args.params will be silently ignored. Consider populating payload["description"] from params (or handling it inside get_connection_config_from_params) when provided, so the CLI behavior matches the advertised params.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Co-authored-by: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py:121
mkdir connectionadvertisesdescriptionas an optional param, but the request payload never copiesargs.params['description']intopayload(andget_connection_config_from_paramsdoes not apply it either). After removing the hardcoded default description, any user-provided description will be silently ignored. Populate the payloaddescriptionfrom params (or haveget_connection_config_from_paramsdo it) when provided so CLI behavior matches the parameter contract.
# Base payload
payload = {
"displayName": connection.short_name,
"connectivityType": connectivityType,
}
if gateway_id:
payload["gatewayId"] = gateway_id
payload = mkdir_utils.get_connection_config_from_params(
payload, con_type, con_type_def, args.params
)
|
@copilot resolve the merge conflicts in this pull request |
📥 Pull Request
✨ Description of new changes
resolving issue #213
This pull request removes the automatic addition of default
"description"fields (such as "Created by fab" or "Imported from fab") from payloads created by the CLI for items, folders, gateways, and connections. Now, descriptions are only included in payloads if explicitly provided by the user. The change is thoroughly tested to ensure backward compatibility and correct behavior.Key changes:
Removal of hardcoded descriptions from payloads:
"description"field from payloads in item, folder, gateway, workspace, and connection creation logic across files likefab_fs_import_item.py,fab_fs_mkdir_item.py,fab_fs_mkdir_folder.py,fab_fs_mkdir_gateway.py,fab_fs_mkdir_workspace.py, and related utility functions. [1] [2] [3] [4] [5] [6] [7]Test updates and new test coverage:
"description"fields in payloads. [1] [2] [3] [4] [5] [6] [7]"description"unless explicitly provided, and that user-supplied descriptions are preserved and forwarded correctly. [1] [2]Connection payload improvements:
get_connection_config_from_params) do not add a"description"by default, and only include it if the user supplies one. [1] [2] [3] [4]These changes make payload construction more predictable and ensure that metadata is only included when intentionally specified by the user.