Skip to content

fix(agent-scan): honor provider timeout_ms in HTTP requests - #625

Open
rwinkelman wants to merge 1 commit into
Tencent:mainfrom
rwinkelman:fix/http-provider-timeout-ms-613
Open

fix(agent-scan): honor provider timeout_ms in HTTP requests#625
rwinkelman wants to merge 1 commit into
Tencent:mainfrom
rwinkelman:fix/http-provider-timeout-ms-613

Conversation

@rwinkelman

Copy link
Copy Markdown
Contributor

Fixes #613

Summary

Honor timeout_ms from provider config in agent-scan HTTP requests.

Test plan

  • Adapter passes timeout to HTTP client

@boy-hack

boy-hack commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Thanks @rwinkelman for fixing #613. Honoring timeout_ms at the httpx client level is the right place, and threading it through all four provider call paths (_call_standard_provider, _call_http_provider, _call_dify_provider, _call_coze_provider) is consistent. _get_timeout_seconds correctly falls back to self.timeout when config is None, so the if config else None guards are correct.

One heads-up before merge: this PR overlaps heavily with the maintainer's in-flight PR #617, which edits the exact same _make_http_request signature and the same four call sites. The only difference is the parameter name — this PR uses timeout=..., while #617 uses timeout_seconds=... (with a richer docstring). Merging both will produce a conflict on adapter.py.

Suggestion:

Otherwise the logic is sound. Once the conflict with #617 is resolved, this is good to merge.

@rwinkelman

Copy link
Copy Markdown
Contributor Author

Grooming pass: timeout_ms honor for HTTP provider. Ping if anything else is needed before merge.

@boy-hack

boy-hack commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the grooming pass, @rwinkelman — the change is still sound. Just a reminder that the merge conflict with #617 stands: my in-flight PR #617 edits the exact same _make_http_request signature and the same four call sites, only with parameter name timeout_seconds (and a richer docstring) instead of timeout. Until one of them lands, they can't both be merged cleanly.

Since #617 is mine, I'd suggest one of:

Either way the logic is correct and checks are green. Want to coordinate on which one we keep?

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.

agent_scan HTTP provider 忽略配置的 timeout_ms,固定使用默认 30 秒超时

2 participants