Skip to content

fix(rate_limiter): forward keyword arguments to synchronous calls - #1027

Draft
yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/rate-limiter-keyword-arguments
Draft

yunyv wants to merge 2 commits into
modelscope:mainfrom
yunyv:fix/rate-limiter-keyword-arguments

Conversation

@yunyv

@yunyv yunyv commented Oct 26, 2026

Copy link
Copy Markdown

Change Summary

RateLimiter.execute() documents support for synchronous functions with keyword arguments, including its url= example. The synchronous branch passes those keywords directly to loop.run_in_executor(), which accepts positional arguments only. The target function is never called and a TypeError is raised.

Bind the callable and its arguments with functools.partial before submitting it to the existing executor. Keep asynchronous call handling, pacing, concurrency and exception propagation unchanged.

Add nine offline regression/control cases covering mixed positional and keyword arguments, keyword-only functions, an executor-named target keyword, existing partials, synchronous calls with no keywords, exception propagation and permit reuse, and asynchronous keyword calls. Tests use the real executor and confirm synchronous functions run off the event-loop thread.

This does not change the synchronous bookkeeping or adaptive-rate logic in #923, or interrupted context entry in #1016.

Related issue number

No linked issue; identified from the documented public API.

Validation

  • Normal package imports on Python 3.12: nine new tests on main produce 5 failures and 4 passes; all nine pass with the fix.
  • New tests plus adjacent retry and task-manager tests: 54 passed, 1 deselected. The excluded transcript-path assertion also fails on unchanged main; the unfiltered patched run was 54 passed, 1 failed.
  • Configured changed-file pre-commit hooks, compilation and git diff --check: passed.
  • Full-repository pre-commit was run on both baseline source and the fix; both report existing formatting/lint issues in unrelated files. All unrelated auto-formatting changes were restored. No full-repository lint-pass claim.
  • Full test suite and live external-service tests were not run. No external API requests were made by these tests.

Checklist

  • The pull request title summarizes the change.
  • Unit tests for the change exist.
  • Full-repository pre-commit passes (baseline issues described above; changed-file hooks pass).
  • Existing API documentation remains accurate; no new option or command is introduced.

AI assistance was used for investigation, implementation and regression tests.

yunyv added 2 commits October 26, 2026 11:37
Bind synchronous call arguments with functools.partial before using the existing executor.
Add nine offline regression and control cases using the real executor, including keyword-only calls and exception propagation.

This branch has not been deployed

No deployments
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.

1 participant