Skip to content

Commit 50f8620

Browse files
aviatcoaviat cohen
andauthored
fix: Set the creation‑method parameters to be optional (#123)
Co-authored-by: aviat cohen <aviatcohen@microsoft.com>
1 parent d82712a commit 50f8620

3 files changed

Lines changed: 181 additions & 32 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: fixed
2+
body: Set the creation‑method parameters to be optional
3+
time: 2026-01-18T09:45:10.759002358Z
4+
custom:
5+
Author: aviatco
6+
AuthorLink: https://github.com/aviatco

src/fabric_cli/utils/fab_cmd_mkdir_utils.py

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -545,36 +545,39 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
545545
parsed_params = []
546546
missing_params = []
547547
if not provided_params:
548-
# Get required and optional parameters from the creation method
549-
req_params_str = ", ".join(
550-
[p["name"] for p in creation_method["parameters"] if p["required"]]
551-
)
552-
opt_params_str = ", ".join(
553-
[p["name"] for p in creation_method["parameters"] if not p["required"]]
554-
)
555-
raise FabricCLIError(
556-
f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}",
557-
fab_constant.ERROR_INVALID_INPUT,
558-
)
559-
for param in creation_method["parameters"]:
560-
p_name = param["name"]
561-
if p_name.lower() not in provided_params and param["required"]:
562-
c_method = creation_method["name"]
563-
missing_params.append(p_name)
564-
if p_name.lower() in provided_params:
565-
parsed_params.append(
566-
{
567-
"dataType": param["dataType"],
568-
"name": p_name,
569-
"value": provided_params[p_name.lower()],
570-
}
548+
# Check if the creation method actually requires parameters
549+
required_params = [p["name"] for p in creation_method["parameters"] if p["required"]]
550+
if required_params:
551+
# Get required and optional parameters from the creation method
552+
req_params_str = ", ".join(required_params)
553+
opt_params_str = ", ".join(
554+
[p["name"] for p in creation_method["parameters"] if not p["required"]]
571555
)
572-
for param in provided_params:
573-
if param not in [p["name"].lower() for p in creation_method["parameters"]]:
574-
c_method = creation_method["name"]
575-
utils_ui.print_warning(
576-
f"Parameter {param} is not used by the creation method {c_method} and will be ignored"
556+
raise FabricCLIError(
557+
f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}",
558+
fab_constant.ERROR_INVALID_INPUT,
577559
)
560+
# If no required parameters, continue with empty parsed_params
561+
else:
562+
for param in creation_method["parameters"]:
563+
p_name = param["name"]
564+
if p_name.lower() not in provided_params and param["required"]:
565+
c_method = creation_method["name"]
566+
missing_params.append(p_name)
567+
if p_name.lower() in provided_params:
568+
parsed_params.append(
569+
{
570+
"dataType": param["dataType"],
571+
"name": p_name,
572+
"value": provided_params[p_name.lower()],
573+
}
574+
)
575+
for param in provided_params:
576+
if param not in [p["name"].lower() for p in creation_method["parameters"]]:
577+
c_method = creation_method["name"]
578+
utils_ui.print_warning(
579+
f"Parameter {param} is not used by the creation method {c_method} and will be ignored"
580+
)
578581

579582
if missing_params:
580583
missing_params_str = ", ".join(missing_params)
@@ -586,8 +589,11 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
586589
connection_request["connectionDetails"] = {
587590
"type": con_type,
588591
"creationMethod": creation_method["name"],
589-
"parameters": parsed_params,
590592
}
593+
594+
# Only add parameters if there are any
595+
if parsed_params:
596+
connection_request["connectionDetails"]["parameters"] = parsed_params
591597

592598
"""
593599
Check that the provided credential type is supported by the connection type:
@@ -659,10 +665,12 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
659665
"skipTestConnection": skipTestConnection,
660666
"credentials": connection_params,
661667
}
662-
connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type
663668

669+
connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type
670+
664671
if is_on_premises_gateway:
665-
connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get("values")
672+
connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get(
673+
"values")
666674

667675
return connection_request
668676

tests/test_utils/test_fab_cmd_mkdir_utils.py

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,142 @@
99
from fabric_cli.core import fab_constant
1010
from fabric_cli.core.fab_exceptions import FabricCLIError
1111
from fabric_cli.errors import ErrorMessages
12-
from fabric_cli.utils.fab_cmd_mkdir_utils import find_mpe_connection
12+
from fabric_cli.utils.fab_cmd_mkdir_utils import (
13+
find_mpe_connection,
14+
get_connection_config_from_params,
15+
)
16+
17+
18+
def test_fabric_data_pipelines_workspace_identity_no_params_success():
19+
"""Test FabricDataPipelines with WorkspaceIdentity credential type when no parameters are required."""
20+
# Arrange
21+
payload = {
22+
"description": "Created by fab",
23+
"displayName": "test-connection",
24+
"connectivityType": "ShareableCloud"
25+
}
26+
27+
con_type = "FabricDataPipelines"
28+
con_type_def = {
29+
"type": "FabricDataPipelines",
30+
"creationMethods": [
31+
{
32+
"name": "FabricDataPipelines.Actions",
33+
"parameters": [] # No parameters required for this creation method
34+
}
35+
],
36+
"supportedCredentialTypes": ["WorkspaceIdentity"]
37+
}
38+
39+
params = {
40+
"connectiondetails": {
41+
"type": "FabricDataPipelines",
42+
"creationmethod": "FabricDataPipelines.Actions"
43+
# No parameters provided since none are required
44+
},
45+
"credentialdetails": {
46+
"type": "WorkspaceIdentity"
47+
# No credential parameters provided since WorkspaceIdentity doesn't require any
48+
}
49+
}
50+
51+
result = get_connection_config_from_params(payload, con_type, con_type_def, params)
52+
53+
# Assert
54+
assert result["privacyLevel"] == "None"
55+
assert result["connectionDetails"]["type"] == "FabricDataPipelines"
56+
assert result["connectionDetails"]["creationMethod"] == "FabricDataPipelines.Actions"
57+
assert "parameters" not in result["connectionDetails"]
58+
assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity"
59+
assert len(result["credentialDetails"]["credentials"].keys()) == 1
60+
61+
62+
def test_connection_with_required_params_missing_failure():
63+
"""Test that connection creation fails when required parameters are missing."""
64+
# Arrange
65+
payload = {
66+
"description": "Created by fab",
67+
"displayName": "test-connection",
68+
"connectivityType": "ShareableCloud"
69+
}
70+
71+
con_type = "SQL"
72+
con_type_def = {
73+
"type": "SQL",
74+
"creationMethods": [
75+
{
76+
"name": "SQL",
77+
"parameters": [
78+
{"name": "server", "required": True, "dataType": "Text"},
79+
{"name": "database", "required": True, "dataType": "Text"}
80+
]
81+
}
82+
],
83+
"supportedCredentialTypes": ["Basic"]
84+
}
85+
86+
params = {
87+
"connectiondetails": {
88+
"type": "SQL",
89+
"creationmethod": "SQL"
90+
# No parameters provided, but they are required
91+
},
92+
"credentialdetails": {
93+
"type": "Basic",
94+
"username": "testuser",
95+
"password": "testpass"
96+
}
97+
}
98+
99+
with pytest.raises(FabricCLIError) as exc_info:
100+
get_connection_config_from_params(payload, con_type, con_type_def, params)
101+
102+
assert "Parameters are required for the connection creation method" in str(exc_info.value.message)
103+
assert "server, database" in str(exc_info.value.message)
104+
105+
106+
def test_workspace_identity_with_unsupported_params_ignored_success():
107+
"""Test that WorkspaceIdentity ignores unsupported credential parameters with warning."""
108+
# Arrange
109+
payload = {
110+
"description": "Created by fab",
111+
"displayName": "test-connection",
112+
"connectivityType": "ShareableCloud"
113+
}
114+
115+
con_type = "FabricDataPipelines"
116+
con_type_def = {
117+
"type": "FabricDataPipelines",
118+
"creationMethods": [
119+
{
120+
"name": "FabricDataPipelines.Actions",
121+
"parameters": []
122+
}
123+
],
124+
"supportedCredentialTypes": ["WorkspaceIdentity"]
125+
}
126+
127+
params = {
128+
"connectiondetails": {
129+
"type": "FabricDataPipelines",
130+
"creationmethod": "FabricDataPipelines.Actions"
131+
},
132+
"credentialdetails": {
133+
"type": "WorkspaceIdentity",
134+
"username": "should_be_ignored", # This should be ignored for WorkspaceIdentity
135+
"password": "should_be_ignored" # This should be ignored for WorkspaceIdentity
136+
}
137+
}
138+
139+
# Act
140+
with patch('fabric_cli.utils.fab_ui.print_warning') as mock_warning:
141+
result = get_connection_config_from_params(payload, con_type, con_type_def, params)
142+
143+
mock_warning.assert_called_once()
144+
assert "username" in str(mock_warning.call_args)
145+
assert "password" in str(mock_warning.call_args)
146+
147+
assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity"
13148

14149

15150
class TestFindMpeConnection:

0 commit comments

Comments
 (0)