Skip to content

fix: exhaust MovingWindowCallRatePolicy bucket when API reports 0 available calls - #1154

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789428944-fix-moving-window-update
Open

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789428944-fix-moving-window-update

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Overview

👉 TL;DR: When an API tells the connector it has no calls left, the moving-window rate limiter was only recording a single extra call instead of marking the whole window as used, so connectors kept sending requests and hit HTTP 429s.

Specifically, MovingWindowCallRatePolicy.update(available_calls=0, call_reset_ts=None) now fills the InMemoryBucket up to the strictest configured Rate via a new _get_available_calls(now) helper.

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

Changes

  • update() previously computed items_to_add = self._bucket.count() < rates[0].limit (a bool) and passed it as RateItem.weight; InMemoryBucket.put() does items.extend(weight * [item]), so True added exactly one entry. It now computes an integer weight:
    min(rate.limit - count(items with timestamp >= now - rate.interval) for rate in bucket.rates)
    which mirrors the per-rate window check inside put(), so the put fills the tightest window exactly (no-op when already full) and is accepted for multi-rate buckets. bucket.count() was also wrong because it includes entries outside the shortest window (leak() only drops entries older than the longest window).
  • The bool return of bucket.put() is now checked; a warning is logged if the sync fails.
  • No behavior change for available_calls > 0 or when call_reset_ts is provided.

Review Spotlight

Reviewers with limited time, please review first:

Test plan

  • New freezegun-based tests in TestMovingWindowCallRatePolicy: 3-of-10 acquired then update(0, None) blocks the next call and unblocks after the window expires; already-full bucket is a no-op; expired-but-not-leaked entries with two rates; three-rate bucket; positive available_calls / provided call_reset_ts remain no-ops.
  • test_update_available_calls_zero_exhausts_bucket fails on main (assert 4 == 10) and passes with the fix.
  • poetry run pytest unit_tests/sources/streams/test_call_rate.py -q (43 passed), poetry run ruff check ., poetry run ruff format ., poetry run mypy --config-file mypy.ini airbyte_cdk/sources/streams/call_rate.py all pass.

Requested via the /ai-fix workflow on the linked issue.

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

…ilable calls

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/1789428944-fix-moving-window-update#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/1789428944-fix-moving-window-update

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.

Copilot AI 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.

🟢 Approval recommended

No unresolved review issues remain.

Pull request overview

Fixes moving-window rate-limit synchronization when an API reports zero available calls.

Changes:

  • Exhausts the bucket across configured rate windows.
  • Logs failed bucket synchronization.
  • Adds regression coverage for exhaustion and edge cases.
File summaries
File Description
unit_tests/sources/streams/test_call_rate.py Adds coverage for zero-call updates and edge cases.
airbyte_cdk/sources/streams/call_rate.py Implements capacity calculation and bucket exhaustion.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pnilan
Patrick Nilan (pnilan) marked this pull request as ready for review September 14, 2026 23:44
@pnilan
Patrick Nilan (pnilan) requested a review from a team as a code owner September 14, 2026 23:44
@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 393 tests  +5   4 381 ✅ +5   9m 37s ⏱️ -1s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit d2ef79a. ± Comparison against base commit 96a7c0a.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 396 tests  +5   4 384 ✅ +5   14m 4s ⏱️ +3s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit d2ef79a. ± 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.

2 participants