Skip to content

Commit 4a80bef

Browse files
foleydangclaude
andcommitted
fix: remove dead code, fix __del__ safety, make agents.update version explicit
- Remove unused _new_request_id() and import uuid from transport.py - Fix __del__ to wrap is_closed check in try/except for interpreter shutdown safety - Make version a required kwarg in agents.update() instead of auto-retrieving - Move stream error handling from session_events into transport retry loop - Add unit tests for agents.update version contract Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 55d6cf3 commit 4a80bef

4 files changed

Lines changed: 121 additions & 86 deletions

File tree

dashscope/agentstudio/resources/agents.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
AgentVersionListParams,
2424
)
2525

26-
2726
_PATH_AGENTS = "/agents"
2827

2928

@@ -82,6 +81,7 @@ def update(
8281
self,
8382
agent_id: str,
8483
*,
84+
version: int,
8585
name: Optional[str] = None,
8686
description: Optional[str] = None,
8787
model: Optional[str] = None,
@@ -91,9 +91,14 @@ def update(
9191
skills: Optional[Sequence[Mapping[str, Any]]] = None,
9292
metadata: Optional[Mapping[str, Any]] = None,
9393
) -> Agent:
94-
current = self.retrieve(agent_id)
94+
"""Update the latest version of an agent.
95+
96+
``version`` must equal the server's current version;
97+
the server rejects mismatches with HTTP 409.
98+
Retrieve the agent first to obtain the current version.
99+
"""
95100
body = AgentUpdateParams(
96-
version=current.version,
101+
version=version,
97102
name=name,
98103
description=description,
99104
model=model,
@@ -229,6 +234,7 @@ async def update(
229234
self,
230235
agent_id: str,
231236
*,
237+
version: int,
232238
name: Optional[str] = None,
233239
description: Optional[str] = None,
234240
model: Optional[str] = None,
@@ -238,9 +244,14 @@ async def update(
238244
skills: Optional[Sequence[Mapping[str, Any]]] = None,
239245
metadata: Optional[Mapping[str, Any]] = None,
240246
) -> Agent:
241-
current = await self.retrieve(agent_id)
247+
"""Update the latest version of an agent.
248+
249+
``version`` must equal the server's current version;
250+
the server rejects mismatches with HTTP 409.
251+
Retrieve the agent first to obtain the current version.
252+
"""
242253
body = AgentUpdateParams(
243-
version=current.version,
254+
version=version,
244255
name=name,
245256
description=description,
246257
model=model,

dashscope/agentstudio/resources/session_events.py

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
Sequence,
1515
)
1616

17-
from dashscope.agentstudio import exceptions
1817
from dashscope.agentstudio.pagination import (
1918
AsyncCursorPage,
2019
CursorPage,
@@ -132,22 +131,6 @@ def stream(
132131
stream=True,
133132
timeout=timeout or AGENTSTUDIO_DEFAULT_TIMEOUT,
134133
)
135-
if resp.status_code >= 400:
136-
try:
137-
resp.read()
138-
body = resp.json()
139-
except ValueError:
140-
body = {"raw": resp.text}
141-
try:
142-
raise exceptions.from_response(
143-
status_code=resp.status_code,
144-
body=body,
145-
headers=resp.headers,
146-
)
147-
finally:
148-
resp.close()
149-
# Transport is shared -- stream close only closes
150-
# the response, NOT the transport.
151134
return _TypedEventStream(
152135
EventStream(response=resp),
153136
)
@@ -294,22 +277,6 @@ async def stream(
294277
stream=True,
295278
timeout=timeout or AGENTSTUDIO_DEFAULT_TIMEOUT,
296279
)
297-
if resp.status_code >= 400:
298-
try:
299-
await resp.aread()
300-
body = resp.json()
301-
except ValueError:
302-
body = {"raw": resp.text}
303-
try:
304-
raise exceptions.from_response(
305-
status_code=resp.status_code,
306-
body=body,
307-
headers=resp.headers,
308-
)
309-
finally:
310-
await resp.aclose()
311-
# Transport is shared -- stream close only closes
312-
# the response, NOT the transport.
313280
return _AioTypedEventStream(
314281
AsyncEventStream(response=resp),
315282
)

dashscope/agentstudio/transport.py

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
(async) so the rest of the SDK can speak in plain Python data structures.
77
The transport layer is responsible for:
88
9-
* injecting auth / workspace / uid / x-request-id headers,
9+
* injecting auth / workspace / uid headers,
1010
* JSON encoding / decoding the request and response bodies,
1111
* converting non-2xx responses into :class:`AgentStudioError`,
1212
* retry-on-network-error with bounded exponential backoff.
@@ -22,7 +22,6 @@
2222
import random
2323
import socket
2424
import time
25-
import uuid
2625
from dataclasses import dataclass
2726
from typing import Any, Dict, IO, Mapping, Optional, Tuple, Union
2827

@@ -132,16 +131,6 @@ def is_error_payload(payload: Any) -> bool:
132131
return False
133132

134133

135-
def _new_request_id() -> str:
136-
"""Generate ``req_<26-char ULID-ish>``.
137-
138-
We use a UUID4 substring rather than an actual ULID to avoid an extra
139-
runtime dependency – the backend treats this header opaquely.
140-
"""
141-
142-
return "req_" + uuid.uuid4().hex[:26]
143-
144-
145134
def _resolve_timeout(
146135
timeout: Union[float, httpx.Timeout, Tuple[float, float], None],
147136
) -> httpx.Timeout:
@@ -381,31 +370,31 @@ def request( # pylint: disable=too-many-branches
381370
if attempt >= self.max_retries or not should_retry:
382371
raise exceptions.APIConnectionError(str(exc)) from exc
383372
else:
384-
if not stream and _should_retry(
373+
if stream:
374+
if resp.status_code >= 400:
375+
resp.read()
376+
else:
377+
return resp
378+
if _should_retry(
385379
resp.status_code,
386380
resp.headers,
387381
):
388382
last_exc = exceptions.APIStatusError(
389383
f"HTTP {resp.status_code}",
390384
status_code=resp.status_code,
391385
)
392-
if attempt >= self.max_retries:
393-
try:
394-
return self._parse(resp)
395-
finally:
396-
resp.close()
397-
wait = _get_retry_after(resp.headers) or _backoff(attempt)
398-
resp.close()
399-
time.sleep(wait)
400-
attempt += 1
401-
continue
402-
if stream:
403-
return resp
386+
if attempt < self.max_retries:
387+
wait = _get_retry_after(resp.headers) or _backoff(
388+
attempt,
389+
)
390+
resp.close()
391+
time.sleep(wait)
392+
attempt += 1
393+
continue
404394
try:
405395
return self._parse(resp)
406396
finally:
407-
if not stream:
408-
resp.close()
397+
resp.close()
409398
time.sleep(_backoff(attempt))
410399
attempt += 1
411400
raise exceptions.APIConnectionError(
@@ -470,9 +459,9 @@ def __exit__(
470459
self.close()
471460

472461
def __del__(self) -> None:
473-
if self.is_closed:
474-
return
475462
try:
463+
if self.is_closed:
464+
return
476465
self.close()
477466
except Exception:
478467
pass
@@ -605,31 +594,31 @@ async def request( # pylint: disable=too-many-branches
605594
if attempt >= self.max_retries or not should_retry:
606595
raise exceptions.APIConnectionError(str(exc)) from exc
607596
else:
608-
if not stream and _should_retry(
597+
if stream:
598+
if resp.status_code >= 400:
599+
await resp.aread()
600+
else:
601+
return resp
602+
if _should_retry(
609603
resp.status_code,
610604
resp.headers,
611605
):
612606
last_exc = exceptions.APIStatusError(
613607
f"HTTP {resp.status_code}",
614608
status_code=resp.status_code,
615609
)
616-
if attempt >= self.max_retries:
617-
try:
618-
return await self._parse(resp)
619-
finally:
620-
await resp.aclose()
621-
wait = _get_retry_after(resp.headers) or _backoff(attempt)
622-
await resp.aclose()
623-
await asyncio.sleep(wait)
624-
attempt += 1
625-
continue
626-
if stream:
627-
return resp
610+
if attempt < self.max_retries:
611+
wait = _get_retry_after(resp.headers) or _backoff(
612+
attempt,
613+
)
614+
await resp.aclose()
615+
await asyncio.sleep(wait)
616+
attempt += 1
617+
continue
628618
try:
629619
return await self._parse(resp)
630620
finally:
631-
if not stream:
632-
await resp.aclose()
621+
await resp.aclose()
633622
await asyncio.sleep(_backoff(attempt))
634623
attempt += 1
635624
raise exceptions.APIConnectionError(

tests/unit/test_agentstudio_protocol.py

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@
2121
from dashscope.agentstudio import exceptions
2222
from dashscope.agentstudio.transport import is_error_payload, unwrap
2323
from dashscope.agentstudio.types import (
24-
user_message,
25-
user_interrupt,
26-
user_tool_confirmation,
2724
user_custom_tool_result,
2825
user_define_outcome,
26+
user_interrupt,
27+
user_message,
28+
user_tool_confirmation,
2929
)
3030

31-
3231
# ---------------------------------------------------------------------------
3332
# 1. error.{code, message}
3433
# ---------------------------------------------------------------------------
@@ -276,3 +275,72 @@ def test_from_response_spring_default():
276275
}
277276
err = exceptions.from_response(status_code=404, body=body)
278277
assert isinstance(err, exceptions.NotFoundError)
278+
279+
280+
# ---------------------------------------------------------------------------
281+
# 4. agents.update version contract (no auto-retrieve)
282+
# ---------------------------------------------------------------------------
283+
284+
285+
class _RecordingTransport:
286+
"""Minimal transport that records requests and returns a canned agent."""
287+
288+
def __init__(self):
289+
self.calls = []
290+
291+
def request(self, method, path, **kwargs):
292+
self.calls.append({"method": method, "path": path, **kwargs})
293+
from dashscope.agentstudio.transport import APIResponse
294+
295+
return APIResponse(
296+
data={"id": "agent_1", "version": 3, "name": "demo"},
297+
request_id="req_1",
298+
)
299+
300+
301+
def _client_with_recording_transport():
302+
from dashscope.agentstudio import Client
303+
304+
c = Client(api_key="test-key", base_url="http://test")
305+
c.transport = _RecordingTransport()
306+
return c
307+
308+
309+
def test_agents_update_requires_version_kwarg():
310+
"""version is required — omitting it is a TypeError."""
311+
client = _client_with_recording_transport()
312+
with pytest.raises(TypeError):
313+
# pylint: disable=missing-kwoa
314+
client.agents.update( # type: ignore[call-arg]
315+
"agent_1",
316+
name="new-name",
317+
)
318+
319+
320+
def test_agents_update_with_version_sends_body():
321+
"""update() sends POST /agents/{id} with version."""
322+
client = _client_with_recording_transport()
323+
client.agents.update(
324+
"agent_1",
325+
version=3,
326+
name="new-name",
327+
)
328+
assert len(client.transport.calls) == 1
329+
call = client.transport.calls[0]
330+
assert call["method"] == "POST"
331+
assert call["path"] == "/agents/agent_1"
332+
body = call["json"]
333+
assert body["version"] == 3
334+
assert body["name"] == "new-name"
335+
336+
337+
def test_agents_update_does_not_auto_retrieve():
338+
"""SDK must NOT call retrieve() internally."""
339+
client = _client_with_recording_transport()
340+
client.agents.update(
341+
"agent_1",
342+
version=3,
343+
name="new-name",
344+
)
345+
assert len(client.transport.calls) == 1
346+
assert client.transport.calls[0]["method"] == "POST"

0 commit comments

Comments
 (0)