Skip to content

fix(cdk): reorder api_budget initialization before dynamic stream discovery#986

Draft
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775852451-fix-api-budget-ordering-dynamic-streams
Draft

fix(cdk): reorder api_budget initialization before dynamic stream discovery#986
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775852451-fix-api-budget-ordering-dynamic-streams

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

Summary

Moves the set_api_budget() call before self.dynamic_streams evaluation in both ConcurrentDeclarativeSource.streams() and ManifestDeclarativeSource.streams().

Previously, the dynamic_streams property triggered HTTP requests (via HttpComponentsResolver) to discover bases/tables before the rate limiter was configured. This caused connectors like source-airtable (5 req/sec budget) to fire discovery requests with no rate limiting, resulting in 429 errors from the upstream API.

Before (buggy):

stream_configs = self._stream_configs(...) + self.dynamic_streams  # HTTP calls without rate limiting
api_budget_model = self._source_config.get("api_budget")
if api_budget_model:
    self._constructor.set_api_budget(api_budget_model, config)

After (fixed):

api_budget_model = self._source_config.get("api_budget")
if api_budget_model:
    self._constructor.set_api_budget(api_budget_model, config)
stream_configs = self._stream_configs(...) + self.dynamic_streams  # Now rate-limited

Regression tests verify the call ordering in both source implementations.

Resolves https://github.com/airbytehq/oncall/issues/11954

Review & Testing Checklist for Human

  • Verify that set_api_budget has no dependency on stream_configs or dynamic_streams being evaluated first (i.e., the reordering is safe). Inspect ModelToComponentFactory.set_api_budget() to confirm it only reads from source_config and config.
  • Note that the check flow accesses source.dynamic_streams directly via check_stream.py (bypassing streams()), so the check path may still lack rate limiting. Consider whether a separate fix is needed there.
  • Validate that the regression tests would actually fail if the code were reverted to the old ordering (you can temporarily swap the lines locally to confirm).

Notes

  • The fix is identical in both concurrent_declarative_source.py and the legacy manifest_declarative_source.py — just 3 lines moved up in each file.
  • The api_budget feature was added in CDK PR feat(low-code): Add API Budget #314; the ordering bug has existed since that PR was merged.

Link to Devin session: https://app.devin.ai/sessions/09ca1fa188574ee28f8a63182797a061

…covery

Move set_api_budget() call before self.dynamic_streams evaluation in both
ConcurrentDeclarativeSource.streams() and ManifestDeclarativeSource.streams().

Previously, dynamic stream discovery HTTP requests bypassed the configured
rate limiter because set_api_budget() was called after self.dynamic_streams
was evaluated. This caused connectors like source-airtable to hit 429 rate
limit errors during connection checks and syncs.

Resolves airbytehq/oncall#11954

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 and CI 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/1775852451-fix-api-budget-ordering-dynamic-streams#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/1775852451-fix-api-budget-ordering-dynamic-streams

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 014 tests  +2   4 003 ✅ +2   7m 46s ⏱️ +3s
    1 suites ±0      11 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 661cb5c. ± Comparison against base commit 4aaafcf.

@github-actions
Copy link
Copy Markdown

PyTest Results (Full)

4 017 tests  +2   4 005 ✅ +2   10m 46s ⏱️ -13s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 661cb5c. ± Comparison against base commit 4aaafcf.

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.

0 participants