Skip to content

fix: block MovingWindowCallRatePolicy until server reset when quota is exhausted - #1160

Draft
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789429889-call-rate-reset-ts
Draft

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789429889-call-rate-reset-ts

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Overview

👉 TL;DR: When an API says it has no calls left and tells us when the quota resets, the moving-window rate policy now stops issuing calls until that reset time instead of ignoring the hint.

Specifically, MovingWindowCallRatePolicy.update(available_calls=0, call_reset_ts=<future>) now records _reset_ts, and try_acquire() raises CallRateLimitHit(time_to_wait=reset_ts - now) until it passes.

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/17244:

Pairs with #1154 (fix for airbytehq/airbyte-internal-issues#17242, which edits the adjacent call_reset_ts is None branch of the same method):

Changes

  • MovingWindowCallRatePolicy.__init__: new _reset_ts: Optional[datetime] field.
  • MovingWindowCallRatePolicy.update(): when available_calls == 0 and call_reset_ts is in the future, store it as _reset_ts; a past call_reset_ts is a no-op. The local bucket is not touched, so configured local rates keep being enforced independently.
  • MovingWindowCallRatePolicy.try_acquire(): before consulting the bucket, raise CallRateLimitHit(rate="api-reset", time_to_wait=_reset_ts - now) while now < _reset_ts; clear _reset_ts once the reset time has passed. APIBudget._do_acquire already sleeps for time_to_wait when block=True, so blocking callers wait exactly until the server reset.
  • __str__ now includes reset_ts=....
  • Tests in unit_tests/sources/streams/test_call_rate.py: block-until-reset (with time_to_wait decreasing over frozen time), past reset ts no-op, local rate still enforced after reset, available_calls=0 without reset ts unchanged, positive available_calls with reset ts no-op, and an end-to-end HttpAPIBudget.update_from_response test with a 429 carrying ratelimit-remaining: 0 + ratelimit-reset.

Review Spotlight

Reviewers with limited time, please review first:

  • airbyte_cdk/sources/streams/call_rate.pytry_acquire() reset gate and the new update() branch (top of the method).

Relationship to #1154

The change is placed above the existing call_reset_ts is None branch and does not modify its lines; a local git merge of this branch with #1154 in either order is conflict-free. The stale TODO comment about this case was deliberately left in place to avoid a merge conflict with #1154 and can be removed once both land. This PR does not change the call_reset_ts is None behavior (airbytehq/airbyte-internal-issues#17242).

Test plan

  • poetry run pytest unit_tests/sources/streams/test_call_rate.py -q — 44 passed
  • New reset-ts tests fail on main (3 failed) and pass with this change
  • poetry run ruff check ., poetry run ruff format ., poetry run mypy --config-file mypy.ini airbyte_cdk/sources/streams/call_rate.py — clean

Link to Devin session: https://app.devin.ai/sessions/0f50fc8284f14af4a26c10516ed5d34b
Open in Devin Desktop: https://app.devin.ai/desktop/session/0f50fc8284f14af4a26c10516ed5d34b?variant=devin

…s exhausted

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1789429889-call-rate-reset-ts#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1789429889-call-rate-reset-ts

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 394 tests  +6   4 382 ✅ +6   9m 33s ⏱️ -5s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 49db636. ± Comparison against base commit 96a7c0a.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 397 tests  +6   4 385 ✅ +6   14m 18s ⏱️ +17s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 49db636. ± Comparison against base commit 96a7c0a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant