Skip to content

fix(models): give QwenVLModel a timeout, wired to DashScope's request_timeout - #193

Merged
XiaoBoAI merged 1 commit into
agentscope-ai:mainfrom
AmirF194:fix/qwen-vl-model-timeout-3379
Sep 7, 2026
Merged

fix(models): give QwenVLModel a timeout, wired to DashScope's request_timeout#193
XiaoBoAI merged 1 commit into
agentscope-ai:mainfrom
AmirF194:fix/qwen-vl-model-timeout-3379

Conversation

@AmirF194

@AmirF194 AmirF194 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

QwenVLModel is the one model class with no timeout knob. Every DashScope call was pinned to the SDK's own 300s default, and passing timeout= to the constructor raised a TypeError.

Worth flagging: the fix isn't just "add a timeout kwarg like OpenAIChatModel does." DashScope's MultiModalConversation.call() accepts a timeout= kwarg but drops it into the request body as an unused parameter; the actual socket timeout only applies through request_timeout=. I checked this by patching HttpRequest.__init__ and confirming timeout=5.0 never reaches it, while request_timeout=5.0 does. So the constructor still takes timeout for parity with the sibling classes, but generate() forwards it as request_timeout internally.

I didn't add max_retries alongside it. DashScope's HTTP layer for this call has no retry mechanism at all, so a max_retries param would be exactly the kind of silent no-op this PR is fixing.

Added tests/models/test_qwen_vl_model.py (didn't exist yet) covering timeout set, unset, and explicit None, each asserting request_timeout (never timeout) is what reaches the mocked call. Ran the full tests/models/ suite on python 3.11, 57 passed / 3 skipped (integration tests need live keys), black and isort clean.

@XiaoBoAI
XiaoBoAI merged commit 5439e9f into agentscope-ai:main Sep 7, 2026
@AmirF194
AmirF194 deleted the fix/qwen-vl-model-timeout-3379 branch September 7, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants