Skip to content

Reject non-positive parallel processor limits - #3013

Open
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/parallel-processor-positive-limits
Open

Reject non-positive parallel processor limits#3013
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/parallel-processor-positive-limits

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

  • Reject non-positive request and token rate limits before the parallel processor enters its scheduling loop.
  • Require at least one request attempt so failed requests cannot be requeued indefinitely with a negative attempt counter.
  • Keep all valid positive limits and retry behavior unchanged.

Motivation

process_api_requests_from_file() assumes its scheduling limits are positive, but currently accepts zero and negative values.

That can make the processor fail to terminate:

  • max_requests_per_minute <= 0 means request capacity never reaches the >= 1 scheduling condition.
  • max_tokens_per_minute <= 0 means token capacity cannot satisfy a positive request cost.
  • max_attempts == 0 is decremented to -1 before the first call; if that call fails, if self.attempts_left: treats the negative value as truthy and requeues the request indefinitely.

Failing at the processor boundary prevents these hang/infinite-retry states without changing normal throttling behavior.

Closes #3012.

Validation

  • positive request/token limits and max_attempts >= 1 -> existing behavior unchanged
  • zero or negative request limit -> immediate ValueError
  • zero or negative token limit -> immediate ValueError
  • max_attempts < 1 -> immediate ValueError
  • validation occurs before opening the request file or entering the scheduling loop

Self-review

  • Final diff is seven added lines in one file.
  • Restored an accidentally omitted documentation line before opening the PR; final compare contains no unrelated changes.
  • No API request schema, throttling formula, retry behavior for valid inputs, dependency, notebook, or registry changes.
  • Searched open PRs and found no competing fix.

Maintainers may modify the branch if needed.

@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner August 21, 2026 16:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d2ce14743

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +122 to +123
if max_requests_per_minute <= 0:
raise ValueError("max_requests_per_minute must be positive")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject request rates below one

When a direct caller passes a positive fractional rate such as 0.5—which is permitted by the float annotation and this validation—the request capacity is initialized and permanently capped below 1, so the scheduling condition available_request_capacity >= 1 can never succeed and a nonempty job runs indefinitely. Reject values below 1, or adjust the token-bucket logic to support fractional request rates.

Useful? React with 👍 / 👎.

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.

Parallel API processor can hang with non-positive limits

1 participant