Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/fixed-20260118-094510.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: fixed
body: Set the creation‑method parameters to be optional
time: 2026-01-18T09:45:10.759002358Z
custom:
Author: aviatco
AuthorLink: https://github.com/aviatco
70 changes: 39 additions & 31 deletions src/fabric_cli/utils/fab_cmd_mkdir_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,36 +545,39 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
parsed_params = []
missing_params = []
if not provided_params:
# Get required and optional parameters from the creation method
req_params_str = ", ".join(
[p["name"] for p in creation_method["parameters"] if p["required"]]
)
opt_params_str = ", ".join(
[p["name"] for p in creation_method["parameters"] if not p["required"]]
)
raise FabricCLIError(
f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}",
fab_constant.ERROR_INVALID_INPUT,
)
for param in creation_method["parameters"]:
p_name = param["name"]
if p_name.lower() not in provided_params and param["required"]:
c_method = creation_method["name"]
missing_params.append(p_name)
if p_name.lower() in provided_params:
parsed_params.append(
{
"dataType": param["dataType"],
"name": p_name,
"value": provided_params[p_name.lower()],
}
# Check if the creation method actually requires parameters
required_params = [p["name"] for p in creation_method["parameters"] if p["required"]]
if required_params:
# Get required and optional parameters from the creation method
req_params_str = ", ".join(required_params)
opt_params_str = ", ".join(
[p["name"] for p in creation_method["parameters"] if not p["required"]]
)
for param in provided_params:
if param not in [p["name"].lower() for p in creation_method["parameters"]]:
c_method = creation_method["name"]
utils_ui.print_warning(
f"Parameter {param} is not used by the creation method {c_method} and will be ignored"
raise FabricCLIError(
f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}",
fab_constant.ERROR_INVALID_INPUT,
)
# If no required parameters, continue with empty parsed_params
else:
for param in creation_method["parameters"]:
p_name = param["name"]
if p_name.lower() not in provided_params and param["required"]:
c_method = creation_method["name"]
missing_params.append(p_name)
if p_name.lower() in provided_params:
parsed_params.append(
{
"dataType": param["dataType"],
"name": p_name,
"value": provided_params[p_name.lower()],
}
)
for param in provided_params:
if param not in [p["name"].lower() for p in creation_method["parameters"]]:
c_method = creation_method["name"]
utils_ui.print_warning(
f"Parameter {param} is not used by the creation method {c_method} and will be ignored"
)

if missing_params:
missing_params_str = ", ".join(missing_params)
Expand All @@ -586,8 +589,11 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
connection_request["connectionDetails"] = {
"type": con_type,
"creationMethod": creation_method["name"],
"parameters": parsed_params,
}

# Only add parameters if there are any
if parsed_params:
connection_request["connectionDetails"]["parameters"] = parsed_params

"""
Check that the provided credential type is supported by the connection type:
Expand Down Expand Up @@ -659,10 +665,12 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params):
"skipTestConnection": skipTestConnection,
"credentials": connection_params,
}
connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type

connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type

if is_on_premises_gateway:
connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get("values")
connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get(
"values")

return connection_request

Expand Down
137 changes: 136 additions & 1 deletion tests/test_utils/test_fab_cmd_mkdir_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,142 @@
from fabric_cli.core import fab_constant
from fabric_cli.core.fab_exceptions import FabricCLIError
from fabric_cli.errors import ErrorMessages
from fabric_cli.utils.fab_cmd_mkdir_utils import find_mpe_connection
from fabric_cli.utils.fab_cmd_mkdir_utils import (
find_mpe_connection,
get_connection_config_from_params,
)


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"
}

con_type = "FabricDataPipelines"
con_type_def = {
"type": "FabricDataPipelines",
"creationMethods": [
{
"name": "FabricDataPipelines.Actions",
"parameters": [] # No parameters required for this creation method
}
],
"supportedCredentialTypes": ["WorkspaceIdentity"]
}

params = {
"connectiondetails": {
"type": "FabricDataPipelines",
"creationmethod": "FabricDataPipelines.Actions"
# No parameters provided since none are required
},
"credentialdetails": {
"type": "WorkspaceIdentity"
# No credential parameters provided since WorkspaceIdentity doesn't require any
}
}

result = get_connection_config_from_params(payload, con_type, con_type_def, params)

# Assert
assert result["privacyLevel"] == "None"
assert result["connectionDetails"]["type"] == "FabricDataPipelines"
assert result["connectionDetails"]["creationMethod"] == "FabricDataPipelines.Actions"
assert "parameters" not in result["connectionDetails"]
assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity"
assert len(result["credentialDetails"]["credentials"].keys()) == 1


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"
}

con_type = "SQL"
con_type_def = {
"type": "SQL",
"creationMethods": [
{
"name": "SQL",
"parameters": [
{"name": "server", "required": True, "dataType": "Text"},
{"name": "database", "required": True, "dataType": "Text"}
]
}
],
"supportedCredentialTypes": ["Basic"]
}

params = {
"connectiondetails": {
"type": "SQL",
"creationmethod": "SQL"
# No parameters provided, but they are required
},
"credentialdetails": {
"type": "Basic",
"username": "testuser",
"password": "testpass"
}
}

with pytest.raises(FabricCLIError) as exc_info:
get_connection_config_from_params(payload, con_type, con_type_def, params)

assert "Parameters are required for the connection creation method" in str(exc_info.value.message)
assert "server, database" in str(exc_info.value.message)


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"
}

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",
"username": "should_be_ignored", # This should be ignored for WorkspaceIdentity
"password": "should_be_ignored" # This should be ignored for WorkspaceIdentity
}
}

# Act
with patch('fabric_cli.utils.fab_ui.print_warning') as mock_warning:
result = get_connection_config_from_params(payload, con_type, con_type_def, params)

mock_warning.assert_called_once()
assert "username" in str(mock_warning.call_args)
assert "password" in str(mock_warning.call_args)

assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity"


class TestFindMpeConnection:
Expand Down