Skip to content

Commit f0db42c

Browse files
author
zhansheng.lzs
committed
style: satisfy pre-commit and fix a Python 3.9 test failure
The pre-commit workflow failed on `add-trailing-comma`, `black` and `pylint`, and the 3.9 unit-test job failed on one new test. pylint: - `_handle_response` was at 52/50 statements. The SSE parse-failure branch it gained was a near-copy of the one in `call()`, so both now share `_sse_parse_failure_response`, which brings it back under the limit instead of widening it. - Four test modules drive private request handlers on purpose; they now carry the module-level `protected-access` disable the other 18 test modules use. - `test_error_detail_preserved` had a bare `yield` after `raise` purely to make the closure an async generator, which reads as unreachable code. It now yields first and raises after, matching the neighbouring test. - `test_agentstudio_protocol` used `import dashscope.agentstudio as ...`. Python 3.9: - `test_record_is_attributed_to_the_caller` read `caplog.records`, which was empty because the SDK logger does not propagate and caplog attaches at the root. It now attaches a collecting handler to the module's own logger, so the stacklevel attribution is verified on every supported version. Tests: 1174 passed, 6 skipped. All pre-commit hooks pass.
1 parent ddc70d9 commit f0db42c

8 files changed

Lines changed: 104 additions & 53 deletions

‎dashscope/api_entities/http_request.py‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,36 @@ def _send_with_retry(
166166
return send()
167167

168168

169+
def _sse_parse_failure_response(
170+
status: int,
171+
data: str,
172+
request_id: str,
173+
headers: dict,
174+
) -> DashScopeAPIResponse:
175+
"""Build the 500 for an SSE body the SDK cannot parse.
176+
177+
An unparsable body is a server-side defect rather than a bad request, and
178+
the payload is the only evidence of it, so it is kept in ``message``.
179+
180+
``status`` feeds the log line only: the sync and async callers read it from
181+
different attributes.
182+
"""
183+
error_message = data or INTERNAL_ERROR.format_msg()
184+
logger.error(
185+
"Request failed: status=%s, code=%s, message=%s",
186+
status,
187+
INTERNAL_ERROR.error_code,
188+
truncate_error_message(error_message),
189+
)
190+
return DashScopeAPIResponse(
191+
request_id=request_id,
192+
status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
193+
code=INTERNAL_ERROR.error_code,
194+
message=error_message,
195+
headers=headers,
196+
)
197+
198+
169199
class HttpRequest(AioBaseRequest):
170200
def __init__(
171201
self,
@@ -434,20 +464,11 @@ async def _handle_aio_response( # pylint: disable=too-many-branches, too-many-s
434464
if "request_id" in msg:
435465
request_id = msg["request_id"]
436466
except json.JSONDecodeError:
437-
error_code = INTERNAL_ERROR.error_code
438-
error_message = data or INTERNAL_ERROR.format_msg()
439-
logger.error(
440-
"Request failed: status=%s, code=%s, message=%s",
467+
yield _sse_parse_failure_response(
441468
response.status,
442-
error_code,
443-
truncate_error_message(error_message),
444-
)
445-
yield DashScopeAPIResponse(
446-
request_id=request_id,
447-
status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
448-
code=error_code,
449-
message=error_message,
450-
headers=headers,
469+
data,
470+
request_id,
471+
headers,
451472
)
452473
continue
453474
if is_error:
@@ -555,21 +576,11 @@ def _handle_response( # pylint: disable=too-many-branches
555576
if "request_id" in msg:
556577
request_id = msg["request_id"]
557578
except json.JSONDecodeError:
558-
error_code = INTERNAL_ERROR.error_code
559-
error_message = data or INTERNAL_ERROR.format_msg()
560-
logger.error(
561-
"Request failed: status=%s, code=%s, message=%s",
579+
yield _sse_parse_failure_response(
562580
response.status_code,
563-
error_code,
564-
truncate_error_message(error_message),
565-
)
566-
yield DashScopeAPIResponse(
567-
request_id=request_id,
568-
status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
569-
output=None,
570-
code=error_code,
571-
message=error_message,
572-
headers=headers,
581+
data,
582+
request_id,
583+
headers,
573584
)
574585
continue
575586
if is_error:

‎dashscope/finetune/agentic_rl.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,11 @@ def _public_exception(public_error, cause: Exception) -> DashScopeException:
156156
``error_code`` instead of the internal one.
157157
"""
158158
summary = f"{type(cause).__name__}: {_exc_message(cause)}"
159-
exc_cls = (
160-
InvalidParameter if public_error == INVALID_REQUEST
161-
else DashScopeException
162-
)
163-
exc = exc_cls(f"{public_error.format_msg()} | Caused by: {summary}")
159+
message = f"{public_error.format_msg()} | Caused by: {summary}"
160+
if public_error == INVALID_REQUEST:
161+
exc = InvalidParameter(message)
162+
else:
163+
exc = DashScopeException(message)
164164
exc.status_code = public_error.status_code
165165
exc.error_code = public_error.error_code
166166
return exc
@@ -192,13 +192,19 @@ def init(self, config_path: Optional[str] = None, **kwargs) -> Self:
192192
Initialize an AgenticRL instance from a YAML configuration file.
193193
"""
194194
try:
195-
self.tuning = TuningModel.load_from_yaml(config_path or "", **kwargs)
195+
self.tuning = TuningModel.load_from_yaml(
196+
config_path or "",
197+
**kwargs,
198+
)
196199
except Exception as e:
197200
_log_internal_error(SDK_AGENTIC_RL_CONFIGURATION_ERROR, e)
198201
# ``load_from_yaml`` reports a missing file and a malformed one
199202
# through the same IOErrorWithCode, and both are the caller's to
200203
# fix. An I/O failure deeper in a request stays a 500.
201-
public_error = _get_public_error(e, input_errors=(IOErrorWithCode,))
204+
public_error = _get_public_error(
205+
e,
206+
input_errors=(IOErrorWithCode,),
207+
)
202208
raise _public_exception(public_error, e) from e
203209

204210
return self

‎tests/unit/test_agentic_rl_init_errors.py‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
PERMISSION_DENIED,
1919
REQUEST_TIMEOUT,
2020
)
21+
from dashscope.finetune import agentic_rl
2122
from dashscope.finetune.agentic_rl import (
2223
AgenticRL,
2324
_log_internal_error,
@@ -125,6 +126,21 @@ def test_log_message_has_no_unfilled_placeholder(self, mock_logger):
125126
assert rendered == "Invalid system configuration detected."
126127

127128

129+
class _Collect(logging.Handler):
130+
"""Collects records straight off the SDK logger.
131+
132+
``caplog`` attaches at the root, and the SDK logger does not propagate, so
133+
it sees nothing here.
134+
"""
135+
136+
def __init__(self, sink):
137+
super().__init__(level=logging.ERROR)
138+
self._sink = sink
139+
140+
def emit(self, record):
141+
self._sink.append(record)
142+
143+
128144
class TestLogInternalError:
129145
def test_keeps_the_positional_log_shape(self):
130146
error_def = MagicMock()
@@ -168,17 +184,22 @@ def test_cause_is_logged_with_a_traceback(self):
168184
assert mock_logger.error.call_args[0][4] is cause
169185
assert mock_logger.error.call_args.kwargs["exc_info"] is True
170186

171-
def test_record_is_attributed_to_the_caller(self, caplog):
187+
def test_record_is_attributed_to_the_caller(self):
172188
"""Every handler logs through this helper, so without the stacklevel
173189
the record's location would be the same line for all five."""
174190
error_def = MagicMock()
175191
error_def.name = "sdk.agentic_rl.SomeFailure"
176192
error_def.format_message.return_value = "rendered"
193+
records = []
194+
handler = _Collect(records)
177195

178-
with caplog.at_level(logging.ERROR, logger="dashscope"):
196+
agentic_rl.logger.addHandler(handler)
197+
try:
179198
_log_internal_error(error_def, RuntimeErrorWithCode("boom"))
199+
finally:
200+
agentic_rl.logger.removeHandler(handler)
180201

181-
assert caplog.records[-1].funcName == (
202+
assert records[-1].funcName == (
182203
"test_record_is_attributed_to_the_caller"
183204
)
184205

‎tests/unit/test_agentstudio_protocol.py‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,10 @@ def test_sdk_originated_codes_use_registry_namespace():
352352
"sdk.agentstudio.StreamClosedError"
353353
)
354354
# Timeout is a specialization of connection failure.
355-
assert issubclass(exceptions.APITimeoutError, exceptions.APIConnectionError)
355+
assert issubclass(
356+
exceptions.APITimeoutError,
357+
exceptions.APIConnectionError,
358+
)
356359

357360

358361
def test_prerelease_error_code_shape_is_accepted():
@@ -367,16 +370,25 @@ def test_prerelease_error_code_shape_is_accepted():
367370
def test_camel_case_request_id_is_accepted():
368371
"""unwrap() already translates requestId on the success path, so the
369372
error path must accept the same spelling."""
370-
body = {"error": {"code": "api_error", "message": "boom"},
371-
"requestId": "req_camel"}
373+
body = {
374+
"error": {"code": "api_error", "message": "boom"},
375+
"requestId": "req_camel",
376+
}
372377
err = exceptions.from_response(status_code=500, body=body)
373378
assert err.request_id == "req_camel"
374379
# snake_case still wins when both are present.
375-
both = {"error": {"code": "api_error"},
376-
"request_id": "req_snake", "requestId": "req_camel"}
377-
assert exceptions.from_response(
378-
status_code=500, body=both,
379-
).request_id == "req_snake"
380+
both = {
381+
"error": {"code": "api_error"},
382+
"request_id": "req_snake",
383+
"requestId": "req_camel",
384+
}
385+
assert (
386+
exceptions.from_response(
387+
status_code=500,
388+
body=both,
389+
).request_id
390+
== "req_snake"
391+
)
380392

381393

382394
def test_recognized_code_outranks_status():
@@ -662,7 +674,7 @@ def test_exception_classes_are_reexported_from_package_root():
662674
other tests here import from the ``exceptions`` submodule, so only this one
663675
catches a repeat.
664676
"""
665-
import dashscope.agentstudio as agentstudio
677+
from dashscope import agentstudio
666678

667679
names = [
668680
"AgentStudioError",

‎tests/unit/test_error_detail_preserved.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ async def failing():
127127

128128
def test_registry_text_is_still_present(self):
129129
async def failing():
130+
yield 1
130131
raise ValueError("bad")
131-
yield # pragma: no cover
132132

133133
response = list(iter_over_async(failing()))[-1]
134134

‎tests/unit/test_error_registry_p0_regressions.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# -*- coding: utf-8 -*-
2+
# pylint: disable=protected-access
23
"""Regression tests for the two P0 defects fixed on dev/errors.
34
45
P0-1: ``AioHttpRequest._handle_response`` yielded the result of the
@@ -129,9 +130,7 @@ def test_no_unawaited_coroutine_warning(self):
129130
asyncio.run(_collect(_make_request(), response))
130131

131132
assert not [
132-
w
133-
for w in caught
134-
if "never awaited" in str(w.message)
133+
w for w in caught if "never awaited" in str(w.message)
135134
], "error response coroutine was not awaited"
136135

137136
def test_registry_has_no_external_field(self):

‎tests/unit/test_error_status_classification.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# -*- coding: utf-8 -*-
2+
# pylint: disable=protected-access
23
"""Failures must be reported with the status they actually mean.
34
45
Three classification defects: a 403 WebSocket handshake was reported as a 401

‎tests/unit/test_sse_parse_error_consistency.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# -*- coding: utf-8 -*-
2+
# pylint: disable=protected-access
23
"""All four SSE parse-failure paths must report the same error.
34
45
A body the SDK cannot parse is a server-side defect, not a bad request, so
@@ -51,9 +52,9 @@ def __init__(self):
5152

5253

5354
def _assert_unified(rsp):
54-
assert rsp.status_code == HTTPStatus.INTERNAL_SERVER_ERROR, (
55-
"an unparsable server body must not be reported as a client error"
56-
)
55+
assert (
56+
rsp.status_code == HTTPStatus.INTERNAL_SERVER_ERROR
57+
), "an unparsable server body must not be reported as a client error"
5758
assert rsp.code == INTERNAL_ERROR.error_code
5859
assert MALFORMED in rsp.message, "the raw payload must reach the caller"
5960
assert "response data" not in rsp.message

0 commit comments

Comments
 (0)