fix: initialize FixedWindowCallRatePolicy next_reset_ts to 1 minute instead of 10 days#989
Open
devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Open
Conversation
… instead of 10 days Previously, the initial reset timestamp was set to 10 days from now. If the API did not return a ratelimit-reset header (e.g. only retry-after), this value was never updated, causing the rate limiter to sleep for ~10 days on a 429 response instead of respecting the configured period. This caused a deadlock where the heartbeat monitor would kill the process after 1.5 hours of no records emitted. Now the initial reset timestamp defaults to now + period (e.g. 1 minute), so the window resets promptly even when the API does not provide reset headers. Also updated the FixedWindowCallRatePolicy component description to document the default reset behavior when no ratelimit-reset header is present. Resolves: airbytehq/oncall#11924 Co-Authored-By: bot_apk <apk@cognition.ai>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou 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/1776356495-fix-fixed-window-rate-policy-default#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/1776356495-fix-fixed-window-rate-policy-defaultPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Changed from using the period variable to a hardcoded 1 minute default as explicitly requested. Co-Authored-By: bot_apk <apk@cognition.ai>
devin-ai-integration
Bot
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes the initial
next_reset_tsforFixedWindowCallRatePolicyfromnow + 10 daystonow + 1 minute. The 10-day default caused a deadlock when the API did not return aratelimit-resetheader: on a 429 response, the rate limiter would sleep for ~10 days instead of recovering promptly. This was the root cause of heartbeat timeouts on the HappyFox connector (oncall#11924).Also updated the
FixedWindowCallRatePolicycomponent description in the YAML schema to document the 1-minute default reset behavior when noratelimit-resetheader is present.The generated Pydantic models (
declarative_component_schema.py) were regenerated viapoe build, which picked up pre-existing drift between the schema and the generated file — these extra changes are not from this PR's edits.Review & Testing Checklist for Human
declarative_component_schema.pyregeneration includes class reordering (ScopesJoinStrategy,BlockSimultaneousSyncsActionmoved), renames (OAuthScope→Scope/OptionalScope), and a type change onStreamGroup.streams(List[str]→List[DeclarativeStream]). These came from schema changes already onmainthat hadn't been regenerated. Confirm these are expected and won't break downstream consumers.next_reset_tsis now always 1 minute regardless of the configuredperiod. This is a safe floor, but confirm this is acceptable for connectors with very short or very long rate limit windows.Suggested test plan: Run an existing connector that uses
FixedWindowCallRatePolicy(with and without aratelimit-resetheader) and verify that (a) the initial window resets after ~1 minute, and (b) subsequent windows still align to server-provided reset timestamps when the header is present.Notes
model_to_component_factory.py:datetime.timedelta(days=10)→datetime.timedelta(minutes=1). Everything else is schema description updates and auto-generated model drift.ratelimit-resetheaders are unaffected — the header value overwritesnext_reset_tson the first response regardless of the initial default.Link to Devin session: https://app.devin.ai/sessions/00d699574c2b4fc084fb0a38d131584e