From a24998c332be2ae9e0e2327cabffe2f44be07b77 Mon Sep 17 00:00:00 2001 From: Aviat Cohen Date: Sun, 19 Apr 2026 10:59:29 +0000 Subject: [PATCH 1/3] remove-hardcoded-description --- .../commands/fs/impor/fab_fs_import_item.py | 1 - .../fs/mkdir/fab_fs_mkdir_connection.py | 1 - .../commands/fs/mkdir/fab_fs_mkdir_folder.py | 1 - .../commands/fs/mkdir/fab_fs_mkdir_gateway.py | 1 - .../commands/fs/mkdir/fab_fs_mkdir_item.py | 1 - .../fs/mkdir/fab_fs_mkdir_workspace.py | 1 - src/fabric_cli/utils/fab_cmd_import_utils.py | 1 - tests/test_core/test_fab_hiearchy.py | 50 ++++++++++-- tests/test_utils/test_fab_cmd_mkdir_utils.py | 80 ++++++++++++++++++- 9 files changed, 120 insertions(+), 17 deletions(-) diff --git a/src/fabric_cli/commands/fs/impor/fab_fs_import_item.py b/src/fabric_cli/commands/fs/impor/fab_fs_import_item.py index b3883699b..09d9ffaa9 100644 --- a/src/fabric_cli/commands/fs/impor/fab_fs_import_item.py +++ b/src/fabric_cli/commands/fs/impor/fab_fs_import_item.py @@ -103,7 +103,6 @@ def _import_create_environment_item( item_payload: dict = { "type": str(item.item_type), - "description": "Imported from fab", "displayName": item.short_name, "folderId": item.folder_id, } diff --git a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py index 78bf2614d..2693c26d4 100644 --- a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py +++ b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_connection.py @@ -110,7 +110,6 @@ def exec(connection: VirtualWorkspaceItem, args: Namespace) -> None: # Base payload payload = { - "description": "Created by fab", "displayName": connection.short_name, "connectivityType": connectivityType, } diff --git a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py index 336d59781..4f00566f0 100644 --- a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py +++ b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py @@ -26,7 +26,6 @@ def exec(folder: Folder, args: Namespace) -> str | None: utils_ui.print_grey(f"Creating a new Folder...") payload = { - "description": "Created by fab", "displayName": foldername, } if parent_folder_id: diff --git a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py index 1354a4478..4c740f379 100644 --- a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py +++ b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py @@ -84,7 +84,6 @@ def exec(gateway: VirtualWorkspaceItem, args: Namespace) -> None: } payload = { - # "description": "Created by fab", "displayName": gateway.short_name, "capacityId": params.get("capacityid"), "inactivityMinutesBeforeSleep": params.get("inactivityminutesbeforesleep", 30), diff --git a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py index a64778a45..58d8bdbd8 100644 --- a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py +++ b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py @@ -78,7 +78,6 @@ def exec(item: Item, args: Namespace) -> str | None: utils.remove_keys_from_dict(params, ["displayname", "type"]) payload = { - "description": "Created by fab", "displayName": item_name, "type": str(item_type), "folderId": item.folder_id, diff --git a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_workspace.py b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_workspace.py index 7caa56dbd..4d347afd3 100644 --- a/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_workspace.py +++ b/src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_workspace.py @@ -68,7 +68,6 @@ def exec(workspace: Workspace, args: Namespace) -> None: utils.remove_keys_from_dict(args.params, ["displayName"]) payload = { - "description": "Created by fab", "displayName": workspace.short_name, } payload.update(args.params) diff --git a/src/fabric_cli/utils/fab_cmd_import_utils.py b/src/fabric_cli/utils/fab_cmd_import_utils.py index ef73525f3..e9f68cdf9 100644 --- a/src/fabric_cli/utils/fab_cmd_import_utils.py +++ b/src/fabric_cli/utils/fab_cmd_import_utils.py @@ -28,7 +28,6 @@ def get_payload_for_item_type( definition = _build_definition(path, input_format) return { "type": str(item.item_type), - "description": "Imported from fab", "folderId": item.folder_id, "displayName": item.short_name, "definition": definition, diff --git a/tests/test_core/test_fab_hiearchy.py b/tests/test_core/test_fab_hiearchy.py index a16e6ac33..7152f4891 100644 --- a/tests/test_core/test_fab_hiearchy.py +++ b/tests/test_core/test_fab_hiearchy.py @@ -408,7 +408,6 @@ def _mock_build(path, resolved_format=""): _expected_payload = { "type": "Notebook", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": {"format": "ipynb", "parts": _base_payload["parts"]}, @@ -429,7 +428,6 @@ def _mock_build(path, resolved_format=""): _expected_payload = { "type": "SparkJobDefinition", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": { @@ -445,7 +443,6 @@ def _mock_build(path, resolved_format=""): _expected_payload = { "type": "SparkJobDefinition", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": { @@ -469,7 +466,6 @@ def _mock_build(path, resolved_format=""): _expected_payload = { "type": "Eventhouse", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": {"parts": _base_payload["parts"]}, @@ -490,7 +486,6 @@ def _mock_build(path, resolved_format=""): _expected_payload = { "type": "Report", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": {"parts": _base_payload["parts"]}, @@ -506,7 +501,6 @@ def _mock_build(path, resolved_format=""): _expected_payload_without_format = { "type": "SemanticModel", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": {"parts": _base_payload["parts"]}, @@ -518,7 +512,6 @@ def _mock_build(path, resolved_format=""): _expected_payload_with_format = { "type": "SemanticModel", - "description": "Imported from fab", "displayName": "item_name", "folderId": None, "definition": { @@ -538,6 +531,49 @@ def _mock_build(path, resolved_format=""): assert get_payload_for_item_type("dummy", report) == _expected_payload +def test_import_payload_does_not_contain_description_by_default(): + """Verify that get_payload_for_item_type never stamps a description in the payload + unless the caller explicitly includes one — i.e. the fix for hardcoded 'Imported from fab'.""" + tenant = Tenant(name="tenant_name", id="0000") + workspace = Workspace( + name="workspace_name", id="workspace_id", parent=tenant, type="Workspace" + ) + + def _mock_build(path, resolved_format=""): + result = {"parts": {}} + if resolved_format: + result["format"] = resolved_format + return result + + item_types = [ + "Notebook", + "SparkJobDefinition", + "EventHouse", + "Report", + "SemanticModel", + "DataPipeline", + "Lakehouse", + ] + + for item_type in item_types: + item = Item( + name="my_item", + id="item_id", + parent=workspace, + item_type=item_type, + ) + with patch( + "fabric_cli.utils.fab_cmd_import_utils._build_definition", + side_effect=_mock_build, + ): + payload = get_payload_for_item_type("dummy", item) + + assert "description" not in payload, ( + f"Payload for {item_type} must not contain 'description' by default, " + f"got: {payload}" + ) + + # ------------------------------------------------------------------- # Tests for _build_definition format handling # ------------------------------------------------------------------- diff --git a/tests/test_utils/test_fab_cmd_mkdir_utils.py b/tests/test_utils/test_fab_cmd_mkdir_utils.py index 987d71e3a..5cdd5b0fa 100644 --- a/tests/test_utils/test_fab_cmd_mkdir_utils.py +++ b/tests/test_utils/test_fab_cmd_mkdir_utils.py @@ -19,7 +19,6 @@ def test_fabric_data_pipelines_workspace_identity_no_params_success(): """Test FabricDataPipelines with WorkspaceIdentity credential type when no parameters are required.""" # Arrange payload = { - "description": "Created by fab", "displayName": "test-connection", "connectivityType": "ShareableCloud" } @@ -63,7 +62,6 @@ def test_connection_with_required_params_missing_failure(): """Test that connection creation fails when required parameters are missing.""" # Arrange payload = { - "description": "Created by fab", "displayName": "test-connection", "connectivityType": "ShareableCloud" } @@ -107,7 +105,6 @@ def test_workspace_identity_with_unsupported_params_ignored_success(): """Test that WorkspaceIdentity ignores unsupported credential parameters with warning.""" # Arrange payload = { - "description": "Created by fab", "displayName": "test-connection", "connectivityType": "ShareableCloud" } @@ -147,6 +144,83 @@ def test_workspace_identity_with_unsupported_params_ignored_success(): assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity" +def test_connection_payload_does_not_contain_description_by_default_success(): + """Verify that get_connection_config_from_params never stamps a description + in the returned payload when the caller does not supply one.""" + payload = { + "displayName": "test-connection", + "connectivityType": "ShareableCloud", + } + + con_type = "FabricDataPipelines" + con_type_def = { + "type": "FabricDataPipelines", + "creationMethods": [ + { + "name": "FabricDataPipelines.Actions", + "parameters": [], + } + ], + "supportedCredentialTypes": ["WorkspaceIdentity"], + } + + params = { + "connectiondetails": { + "type": "FabricDataPipelines", + "creationmethod": "FabricDataPipelines.Actions", + }, + "credentialdetails": { + "type": "WorkspaceIdentity", + }, + } + + result = get_connection_config_from_params( + payload, con_type, con_type_def, params) + + assert "description" not in result, ( + f"Connection payload must not contain 'description' by default, got: {result}" + ) + + +def test_connection_payload_contains_description_when_user_provides_it_success(): + """Verify that when a user explicitly passes description via params it is + forwarded into the payload returned by get_connection_config_from_params.""" + payload = { + "displayName": "test-connection", + "connectivityType": "ShareableCloud", + "description": "My custom description", # user supplied before calling helper + } + + con_type = "FabricDataPipelines" + con_type_def = { + "type": "FabricDataPipelines", + "creationMethods": [ + { + "name": "FabricDataPipelines.Actions", + "parameters": [], + } + ], + "supportedCredentialTypes": ["WorkspaceIdentity"], + } + + params = { + "connectiondetails": { + "type": "FabricDataPipelines", + "creationmethod": "FabricDataPipelines.Actions", + }, + "credentialdetails": { + "type": "WorkspaceIdentity", + }, + } + + result = get_connection_config_from_params( + payload, con_type, con_type_def, params) + + assert result.get("description") == "My custom description", ( + f"Connection payload must preserve a user-supplied description, got: {result}" + ) + + class TestFindMpeConnection: """Test cases for find_mpe_connection function.""" From 6f8ae27d468a3bee4ffca88967e4f550f7cc6bdc Mon Sep 17 00:00:00 2001 From: Aviat Cohen Date: Sun, 19 Apr 2026 11:03:27 +0000 Subject: [PATCH 2/3] add changie log --- .changes/unreleased/fixed-20260419-105923.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/fixed-20260419-105923.yaml diff --git a/.changes/unreleased/fixed-20260419-105923.yaml b/.changes/unreleased/fixed-20260419-105923.yaml new file mode 100644 index 000000000..3392a78fe --- /dev/null +++ b/.changes/unreleased/fixed-20260419-105923.yaml @@ -0,0 +1,6 @@ +kind: fixed +body: Remove created By Fab hardcoded description when create/import items +time: 2026-04-19T10:59:23.659719244Z +custom: + Author: aviatco + AuthorLink: https://github.com/aviatco From 569b763eecab8b1106d4dd4c8f75bb1b98748419 Mon Sep 17 00:00:00 2001 From: aviatco <32952699+aviatco@users.noreply.github.com> Date: Sun, 19 Apr 2026 14:11:41 +0300 Subject: [PATCH 3/3] Update .changes/unreleased/fixed-20260419-105923.yaml Co-authored-by: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com> --- .changes/unreleased/fixed-20260419-105923.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/fixed-20260419-105923.yaml b/.changes/unreleased/fixed-20260419-105923.yaml index 3392a78fe..7a61e1aef 100644 --- a/.changes/unreleased/fixed-20260419-105923.yaml +++ b/.changes/unreleased/fixed-20260419-105923.yaml @@ -1,5 +1,5 @@ kind: fixed -body: Remove created By Fab hardcoded description when create/import items +body: Remove hardcoded description when create/import items time: 2026-04-19T10:59:23.659719244Z custom: Author: aviatco