Skip to content

Commit fabed21

Browse files
authored
fix: pipeline create and update should not leak transport details (#19)
- return clean response - raise any errors that might have occurred
1 parent 0938dab commit fabed21

5 files changed

Lines changed: 56 additions & 55 deletions

File tree

src/deepset_mcp/api/pipeline/models.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,10 @@ class PipelineValidationResult(BaseModel):
5959

6060
valid: bool
6161
errors: list[ValidationError] = []
62+
63+
64+
class NoContentResponse(BaseModel):
65+
"""Response model for an empty response."""
66+
67+
success: bool = True
68+
message: str = "No content"

src/deepset_mcp/api/pipeline/resource.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
from typing import Any
22

33
from deepset_mcp.api.exceptions import UnexpectedAPIError
4-
from deepset_mcp.api.pipeline.models import DeepsetPipeline, PipelineValidationResult, ValidationError
4+
from deepset_mcp.api.pipeline.models import (
5+
DeepsetPipeline,
6+
NoContentResponse,
7+
PipelineValidationResult,
8+
ValidationError,
9+
)
510
from deepset_mcp.api.protocols import AsyncClientProtocol, PipelineResourceProtocol
6-
from deepset_mcp.api.transport import TransportResponse, raise_for_status
11+
from deepset_mcp.api.transport import raise_for_status
712

813

914
class PipelineResource(PipelineResourceProtocol):
@@ -109,7 +114,7 @@ async def get(self, pipeline_name: str, include_yaml: bool = True) -> DeepsetPip
109114

110115
return pipeline
111116

112-
async def create(self, name: str, yaml_config: str) -> TransportResponse:
117+
async def create(self, name: str, yaml_config: str) -> NoContentResponse:
113118
"""Create a new pipeline with a name and YAML config."""
114119
data = {"name": name, "query_yaml": yaml_config}
115120
resp = await self._client.request(
@@ -120,14 +125,14 @@ async def create(self, name: str, yaml_config: str) -> TransportResponse:
120125

121126
raise_for_status(resp)
122127

123-
return resp
128+
return NoContentResponse(message="Pipeline created successfully.")
124129

125130
async def update(
126131
self,
127132
pipeline_name: str,
128133
updated_pipeline_name: str | None = None,
129134
yaml_config: str | None = None,
130-
) -> TransportResponse:
135+
) -> NoContentResponse:
131136
"""Update name and/or YAML config of an existing pipeline."""
132137
# Handle name update first if any
133138
if updated_pipeline_name is not None:
@@ -142,7 +147,7 @@ async def update(
142147
pipeline_name = updated_pipeline_name
143148

144149
if yaml_config is None:
145-
return name_resp
150+
return NoContentResponse(message="Pipeline name updated successfully.")
146151

147152
if yaml_config is not None:
148153
yaml_resp = await self._client.request(
@@ -153,6 +158,11 @@ async def update(
153158

154159
raise_for_status(yaml_resp)
155160

156-
return yaml_resp
161+
if updated_pipeline_name is not None:
162+
response = NoContentResponse(message="Pipeline name and YAML updated successfully.")
163+
else:
164+
response = NoContentResponse(message="Pipeline YAML updated successfully.")
165+
166+
return response
157167

158168
raise ValueError("Either `updated_pipeline_name` or `yaml_config` must be provided.")

src/deepset_mcp/api/protocols.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from types import TracebackType
22
from typing import Any, Protocol, Self
33

4-
from deepset_mcp.api.pipeline.models import DeepsetPipeline, PipelineValidationResult
4+
from deepset_mcp.api.pipeline.models import DeepsetPipeline, NoContentResponse, PipelineValidationResult
55
from deepset_mcp.api.transport import TransportResponse
66

77

@@ -56,7 +56,7 @@ async def list(
5656
"""List pipelines in the configured workspace with optional pagination."""
5757
...
5858

59-
async def create(self, name: str, yaml_config: str) -> Any:
59+
async def create(self, name: str, yaml_config: str) -> NoContentResponse:
6060
"""Create a new pipeline with a name and YAML config."""
6161
...
6262

@@ -65,6 +65,6 @@ async def update(
6565
pipeline_name: str,
6666
updated_pipeline_name: str | None = None,
6767
yaml_config: str | None = None,
68-
) -> Any:
68+
) -> NoContentResponse:
6969
"""Update name and/or YAML config of an existing pipeline."""
7070
...

src/deepset_mcp/tools/pipeline.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,16 @@ async def create_pipeline(
4343
if not validation_response.valid:
4444
return validation_result_to_llm_readable_string(validation_response)
4545

46-
creation_response = await client.pipelines(workspace=workspace).create(
47-
name=pipeline_name, yaml_config=yaml_configuration
48-
)
49-
50-
if creation_response.success:
51-
return f"Pipeline '{pipeline_name}' created successfully."
46+
try:
47+
await client.pipelines(workspace=workspace).create(name=pipeline_name, yaml_config=yaml_configuration)
48+
except ResourceNotFoundError:
49+
return f"There is no workspace named '{workspace}'. Did you mean to configure it?"
50+
except BadRequestError as e:
51+
return f"Failed to create pipeline '{pipeline_name}': {e}"
52+
except UnexpectedAPIError as e:
53+
return f"Failed to create pipeline '{pipeline_name}': {e}"
5254

53-
return f"Failed to create pipeline '{pipeline_name}': {creation_response.text}"
55+
return f"Pipeline '{pipeline_name}' created successfully."
5456

5557

5658
async def update_pipeline(
@@ -96,7 +98,7 @@ async def update_pipeline(
9698
return validation_result_to_llm_readable_string(validation_response)
9799

98100
try:
99-
update_response = await client.pipelines(workspace=workspace).update(
101+
await client.pipelines(workspace=workspace).update(
100102
pipeline_name=pipeline_name, yaml_config=updated_yaml_configuration
101103
)
102104
except ResourceNotFoundError:
@@ -106,7 +108,4 @@ async def update_pipeline(
106108
except UnexpectedAPIError as e:
107109
return f"Failed to update the pipeline '{pipeline_name}': {e}"
108110

109-
if update_response.success:
110-
return f"The pipeline '{pipeline_name}' was successfully updated."
111-
112-
return f"Failed to update the pipeline '{pipeline_name}': {update_response.text}"
111+
return f"The pipeline '{pipeline_name}' was successfully updated."

test/unit/tools/test_pipeline.py

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from datetime import datetime
2-
from typing import Any, Self
2+
from typing import Self
33

44
import pytest
55

@@ -11,6 +11,7 @@
1111
from deepset_mcp.api.pipeline.models import (
1212
DeepsetPipeline,
1313
DeepsetUser,
14+
NoContentResponse,
1415
PipelineServiceLevel,
1516
PipelineValidationResult,
1617
ValidationError,
@@ -32,15 +33,17 @@ def __init__(
3233
list_response: list[DeepsetPipeline] | None = None,
3334
get_response: DeepsetPipeline | None = None,
3435
validate_response: PipelineValidationResult | None = None,
35-
create_response: Any | None = None,
36-
update_response: Any | None = None,
36+
create_response: NoContentResponse | None = None,
37+
update_response: NoContentResponse | None = None,
3738
get_exception: Exception | None = None,
3839
update_exception: Exception | None = None,
40+
create_exception: Exception | None = None,
3941
) -> None:
4042
self._list_response = list_response
4143
self._get_response = get_response
4244
self._validate_response = validate_response
4345
self._create_response = create_response
46+
self._create_exception = create_exception
4447
self._update_response = update_response
4548
self._get_exception = get_exception
4649
self._update_exception = update_exception
@@ -62,7 +65,9 @@ async def validate(self, yaml_config: str) -> PipelineValidationResult:
6265
return self._validate_response
6366
raise NotImplementedError
6467

65-
async def create(self, name: str, yaml_config: str) -> Any:
68+
async def create(self, name: str, yaml_config: str) -> NoContentResponse:
69+
if self._create_exception:
70+
raise self._create_exception
6671
if self._create_response is not None:
6772
return self._create_response
6873
raise NotImplementedError
@@ -72,7 +77,7 @@ async def update(
7277
pipeline_name: str,
7378
updated_pipeline_name: str | None = None,
7479
yaml_config: str | None = None,
75-
) -> Any:
80+
) -> NoContentResponse:
7681
if self._update_exception:
7782
raise self._update_exception
7883
if self._update_response is not None:
@@ -204,27 +209,23 @@ async def test_create_pipeline_handles_validation_failure() -> None:
204209
async def test_create_pipeline_handles_success_and_failure_response() -> None:
205210
valid_result = PipelineValidationResult(valid=True, errors=[])
206211

207-
class Creation:
208-
def __init__(self, success: bool, text: str = "") -> None:
209-
self.success = success
210-
self.text = text
211-
212212
# success
213213
resource_succ = FakePipelineResource(
214214
validate_response=valid_result,
215-
create_response=Creation(success=True),
215+
create_response=NoContentResponse(message="created successfully"),
216216
)
217217
client_succ = FakeClient(resource_succ)
218218
res_succ = await create_pipeline(client_succ, workspace="ws", pipeline_name="p1", yaml_configuration="a: b")
219+
219220
assert "created successfully" in res_succ
220221
# failure
221222
resource_fail = FakePipelineResource(
222223
validate_response=valid_result,
223-
create_response=Creation(success=False, text="bad things"),
224+
create_exception=BadRequestError(message="bad things"),
224225
)
225226
client_fail = FakeClient(resource_fail)
226227
res_fail = await create_pipeline(client_fail, workspace="ws", pipeline_name="p1", yaml_configuration="a: b")
227-
assert "Failed to create pipeline 'p1': bad things" == res_fail
228+
assert "Failed to create pipeline 'p1': bad things (Status Code: 400)" == res_fail
228229

229230

230231
@pytest.mark.asyncio
@@ -373,7 +374,7 @@ async def test_update_pipeline_exceptions_on_update() -> None:
373374

374375

375376
@pytest.mark.asyncio
376-
async def test_update_pipeline_success_and_failure_response() -> None:
377+
async def test_update_pipeline_success_response() -> None:
377378
user = DeepsetUser(user_id="u1", given_name="A", family_name="B")
378379
orig_yaml = "foo: 1"
379380
original = DeepsetPipeline(
@@ -389,14 +390,11 @@ async def test_update_pipeline_success_and_failure_response() -> None:
389390
)
390391
val_ok = PipelineValidationResult(valid=True, errors=[])
391392

392-
class UpdateResp:
393-
def __init__(self, success: bool, text: str = "") -> None:
394-
self.success = success
395-
self.text = text
396-
397393
# success
398394
res_succ = FakePipelineResource(
399-
get_response=original, validate_response=val_ok, update_response=UpdateResp(success=True)
395+
get_response=original,
396+
validate_response=val_ok,
397+
update_response=NoContentResponse(message="successfully updated"),
400398
)
401399
client_succ = FakeClient(res_succ)
402400
r_success = await update_pipeline(
@@ -407,16 +405,3 @@ def __init__(self, success: bool, text: str = "") -> None:
407405
replacement_config_snippet="foo: 2",
408406
)
409407
assert "successfully updated" in r_success.lower()
410-
# failure
411-
res_fail = FakePipelineResource(
412-
get_response=original, validate_response=val_ok, update_response=UpdateResp(success=False, text="nope")
413-
)
414-
client_fail = FakeClient(res_fail)
415-
r_fail = await update_pipeline(
416-
client_fail,
417-
workspace="ws",
418-
pipeline_name="np",
419-
original_config_snippet="foo: 1",
420-
replacement_config_snippet="foo: 2",
421-
)
422-
assert r_fail == "Failed to update the pipeline 'np': nope"

0 commit comments

Comments
 (0)