feat: add weight-based rate limiting support to HttpRequestRegexMatcher#966
Merged
Daryna Ishchenko (darynaishchenko) merged 9 commits intomainfrom Mar 30, 2026
Conversation
Add optional 'cost' field to HttpRequestRegexMatcher that allows specifying per-request cost/weight in rate limiting policies. This enables cost-based rate limiting where different endpoints consume different amounts from a shared budget (e.g., Amplitude's Dashboard REST API). Changes: - YAML schema: add 'cost' field (int or string) to HttpRequestRegexMatcher - Python model: add 'cost' field to HttpRequestRegexMatcher model - call_rate.py: store cost on matcher, add get_cost() to BaseCallRatePolicy, update APIBudget._do_acquire() to use cost as weight instead of hardcoded 1 - model_to_component_factory.py: wire up cost field with interpolation support - Tests: 8 new tests for cost-based rate limiting behavior Backward compatible: cost defaults to None (treated as 1). Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Contributor
🤖 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/1774609130-cost-based-rate-limiting#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/1774609130-cost-based-rate-limitingPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded an optional Changes
Sequence DiagramsequenceDiagram
participant Client
participant APIBudget
participant Policy as BaseCallRatePolicy
participant Matcher as HttpRequestRegexMatcher
Client->>APIBudget: _do_acquire(request)
APIBudget->>Policy: get_weight(request)
Policy->>Matcher: matches(request)?
alt Matcher found with weight
Matcher-->>Policy: returns weight (int)
Policy-->>APIBudget: weight
else No match or weight unset
Policy-->>APIBudget: 1 (default)
end
APIBudget->>Policy: try_acquire(weight)
Policy-->>APIBudget: acquired (true/false)
APIBudget-->>Client: permit or reject
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Would you like me to rename remaining "cost" occurrences to "weight" for consistency, wdyt? 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…quire(weight=...) Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
airbyte_cdk/sources/streams/call_rate.py (1)
627-628: Could you hoistweightout of the retry loop, wdyt?
policy.get_cost(request)is invariant for this call, but right now every retry re-runs matcher selection afterget_matching_policy()has already matched the request. Computing it once before the loop trims an extra match/prepare pass from the hot path.Small cleanup
last_exception = None endpoint = self._extract_endpoint(request) + weight = policy.get_cost(request) if isinstance(policy, BaseCallRatePolicy) else 1 # sometimes we spend all budget before a second attempt, so we have a few more attempts for attempt in range(1, self._maximum_attempts_to_acquire): try: - weight = policy.get_cost(request) if isinstance(policy, BaseCallRatePolicy) else 1 policy.try_acquire(request, weight=weight) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/streams/call_rate.py` around lines 627 - 628, The call to policy.get_cost(request) should be hoisted out of the retry loop because the selected policy from get_matching_policy(request) is invariant for this request; compute weight once after you obtain policy (using BaseCallRatePolicy check) and store it in a local variable, then use that weight in every subsequent policy.try_acquire(request, weight=weight) call inside the retry loop so matcher selection/prepare isn't repeated on each retry.airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
1835-1844: Could you declare the interpolation context forcost, wdyt?The runtime path now supports templated string costs, but this schema still exposes
costas a bare string. Adding the same interpolation context here keeps Builder/docs/schema-driven validation aligned with the new config-driven behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/declarative/declarative_component_schema.yaml` around lines 1835 - 1844, The schema for the "cost" property needs an interpolation context so templated string costs validate the same way at build/docs time as at runtime; update the "cost" entry in declarative_component_schema.yaml (the property named cost under the matcher block shown in the diff) to include the same interpolation context metadata used by other templated string fields in this schema (copy the interpolation context key/value pair used elsewhere for templated strings and attach it to cost), ensuring both "type: string" and the interpolation-aware validation are present.unit_tests/sources/streams/test_call_rate.py (1)
366-455: Could you add one parser-level stringcosttest as well, wdyt?These cases only construct
HttpRequestRegexMatcher(..., cost=<int>)directly. The PR also adds config/interpolatedcostsupport, so the factory/parser path can regress without any of these tests failing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@unit_tests/sources/streams/test_call_rate.py` around lines 366 - 455, Add a parser/factory-level test (e.g., test_parser_cost_string) that verifies a string cost in config is parsed into an integer cost for HttpRequestRegexMatcher: use the project’s config/parser/factory to create a MovingWindowCallRatePolicy (or APIBudget) with a matcher whose cost is provided as a string (e.g., "60"), then assert policy.get_cost(Request(...)) == 60 and/or that APIBudget.acquire_call uses that weight (raise CallRateLimitHit when exceeded). Reference HttpRequestRegexMatcher, MovingWindowCallRatePolicy and APIBudget in the test so the parser path (not direct constructor) is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py`:
- Around line 4390-4395: The cost resolution currently uses
InterpolatedString.create(cost, parameters={}) and doesn't enforce a minimum;
update the string interpolation to pass through the matcher parameters (use the
existing parameters variable instead of {} when calling
InterpolatedString.create) and after resolving/coercing to int (both for str and
non-str branches) clamp the value to at least 1 (e.g., cost = max(1,
int(resolved_cost))). Locate the `cost` handling block around the `cost =
model.cost` assignment and update the `InterpolatedString.create` call plus the
final assignment so `cost` is always an integer >= 1; keep the `eval(config)`
call for evaluation.
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 169-181: Reject non-positive matcher costs by validating the
incoming cost before assigning self._cost: in the constructor where self._cost
is set, check if cost is not None and if cost <= 0 (or not an int/float if you
want stricter typing) and raise a ValueError with a clear message; this prevents
zero or negative weights from flowing into FixedWindowCallRatePolicy and
incorrectly making requests free or reducing _calls_num.
---
Nitpick comments:
In `@airbyte_cdk/sources/declarative/declarative_component_schema.yaml`:
- Around line 1835-1844: The schema for the "cost" property needs an
interpolation context so templated string costs validate the same way at
build/docs time as at runtime; update the "cost" entry in
declarative_component_schema.yaml (the property named cost under the matcher
block shown in the diff) to include the same interpolation context metadata used
by other templated string fields in this schema (copy the interpolation context
key/value pair used elsewhere for templated strings and attach it to cost),
ensuring both "type: string" and the interpolation-aware validation are present.
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 627-628: The call to policy.get_cost(request) should be hoisted
out of the retry loop because the selected policy from
get_matching_policy(request) is invariant for this request; compute weight once
after you obtain policy (using BaseCallRatePolicy check) and store it in a local
variable, then use that weight in every subsequent policy.try_acquire(request,
weight=weight) call inside the retry loop so matcher selection/prepare isn't
repeated on each retry.
In `@unit_tests/sources/streams/test_call_rate.py`:
- Around line 366-455: Add a parser/factory-level test (e.g.,
test_parser_cost_string) that verifies a string cost in config is parsed into an
integer cost for HttpRequestRegexMatcher: use the project’s
config/parser/factory to create a MovingWindowCallRatePolicy (or APIBudget) with
a matcher whose cost is provided as a string (e.g., "60"), then assert
policy.get_cost(Request(...)) == 60 and/or that APIBudget.acquire_call uses that
weight (raise CallRateLimitHit when exceeded). Reference
HttpRequestRegexMatcher, MovingWindowCallRatePolicy and APIBudget in the test so
the parser path (not direct constructor) is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1b2adfec-6da5-4d49-875e-6582e514b3af
📒 Files selected for processing (5)
airbyte_cdk/sources/declarative/declarative_component_schema.yamlairbyte_cdk/sources/declarative/models/declarative_component_schema.pyairbyte_cdk/sources/declarative/parsers/model_to_component_factory.pyairbyte_cdk/sources/streams/call_rate.pyunit_tests/sources/streams/test_call_rate.py
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
4390-4395:⚠️ Potential issue | 🟠 MajorCould we resolve
weightwith matcher parameters here, wdyt?Using
parameters={}means{{ parameters[...] }}weights cannot resolve, even though the rest of this factory usually threads matcher parameters into interpolation.💡 Suggested change
def create_http_request_matcher( self, model: HttpRequestRegexMatcherModel, config: Config, **kwargs: Any ) -> HttpRequestRegexMatcher: weight = model.weight if weight is not None: + interpolation_parameters = kwargs.get("parameters", model.parameters or {}) or {} if isinstance(weight, str): - weight = int(InterpolatedString.create(weight, parameters={}).eval(config)) + weight = int( + InterpolatedString.create( + weight, parameters=interpolation_parameters + ).eval(config=config) + ) else: weight = int(weight)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py` around lines 4390 - 4395, The code resolves model.weight with InterpolatedString.create(..., parameters={}) which prevents matcher parameters from being interpolated; update the call that creates/evaluates weight (the block handling weight from model.weight using InterpolatedString.create and .eval) to pass the real matcher parameters object instead of an empty dict (e.g., parameters=matcher_parameters or the existing parameters variable used elsewhere in this factory) so {{ parameters[...] }} templates are resolved.airbyte_cdk/sources/streams/call_rate.py (1)
169-182:⚠️ Potential issue | 🟠 MajorCould we reject
weight <= 0in the matcher ctor too, wdyt?Direct Python usage bypasses the declarative factory, and
FixedWindowCallRatePolicycurrently treats0as free and negatives as budget-restoring.💡 Suggested change
def __init__( self, method: Optional[str] = None, url_base: Optional[str] = None, url_path_pattern: Optional[str] = None, params: Optional[Mapping[str, Any]] = None, headers: Optional[Mapping[str, Any]] = None, weight: Optional[int] = None, ): @@ - self._weight = weight + if weight is not None and weight < 1: + raise ValueError(f"weight must be >= 1, got {weight}") + self._weight = weight🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/streams/call_rate.py` around lines 169 - 182, The matcher constructor currently accepts weight values <= 0 which leads to unexpected semantics (0 treated as free, negatives as budget-restoring); in the __init__ where the weight parameter is assigned to self._weight validate that if weight is not None it must be > 0 and raise a ValueError otherwise, updating the error message to mention the matcher (weight) argument; ensure the same validation logic is applied where RequestMatcher (or the specific matcher class in call_rate.py) is instantiated so direct Python usage is rejected consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 279-295: get_weight currently scans all _matchers looking for any
weighted matcher, allowing a later matcher to override an earlier match; change
get_weight to stop at the first matcher that matches the request (same
short-circuit behavior as matches): in get_weight, iterate self._matchers and
when matcher(request) is True immediately return matcher.weight if the matcher
is an HttpRequestRegexMatcher with a non-None weight, otherwise return 1 for
that first matching matcher; this ensures get_weight respects the first-match
policy used by matches.
---
Duplicate comments:
In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py`:
- Around line 4390-4395: The code resolves model.weight with
InterpolatedString.create(..., parameters={}) which prevents matcher parameters
from being interpolated; update the call that creates/evaluates weight (the
block handling weight from model.weight using InterpolatedString.create and
.eval) to pass the real matcher parameters object instead of an empty dict
(e.g., parameters=matcher_parameters or the existing parameters variable used
elsewhere in this factory) so {{ parameters[...] }} templates are resolved.
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 169-182: The matcher constructor currently accepts weight values
<= 0 which leads to unexpected semantics (0 treated as free, negatives as
budget-restoring); in the __init__ where the weight parameter is assigned to
self._weight validate that if weight is not None it must be > 0 and raise a
ValueError otherwise, updating the error message to mention the matcher (weight)
argument; ensure the same validation logic is applied where RequestMatcher (or
the specific matcher class in call_rate.py) is instantiated so direct Python
usage is rejected consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3e015976-ee05-4fb2-a685-e9072639b661
📒 Files selected for processing (5)
airbyte_cdk/sources/declarative/declarative_component_schema.yamlairbyte_cdk/sources/declarative/models/declarative_component_schema.pyairbyte_cdk/sources/declarative/parsers/model_to_component_factory.pyairbyte_cdk/sources/streams/call_rate.pyunit_tests/sources/streams/test_call_rate.py
✅ Files skipped from review due to trivial changes (1)
- airbyte_cdk/sources/declarative/declarative_component_schema.yaml
devin-ai-integration
bot
changed the title
Address CodeRabbit review: get_weight() now stops at the first matcher that matches the request, consistent with matches() behavior. If the first matching matcher has a weight, use it; otherwise return 1. Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Reject non-positive weight values in the constructor to prevent weight=0 (free requests) or negative weights (budget restoration) from bypassing rate limits. Adds two tests for validation. Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Validates weight >= 1 after interpolation/casting in the factory, catching invalid weights from manifest config before they reach the HttpRequestRegexMatcher constructor. Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
airbyte_cdk/sources/streams/call_rate.py (1)
281-295: Could we make weight lookup go through the matcher abstraction here, wdyt?This branch only honors
HttpRequestRegexMatcher, so the convenienceHttpRequestMatcherin this module still can't participate in weighted charging even though it delegates to a regex matcher underneath. The declarative path already forwardsweightintoHttpRequestRegexMatcherinairbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:4388-4407; exposingweightonRequestMatcher/HttpRequestMatcherwould keep both entry points aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/streams/call_rate.py` around lines 281 - 295, The get_weight function currently special-cases HttpRequestRegexMatcher; instead, ask matchers for their weight via the matcher abstraction: update get_weight to, for each matcher in self._matchers, call matcher(request) and then obtain the weight by first trying a dedicated method/property on the matcher (e.g., call matcher.get_weight(request) if present or read matcher.weight via getattr), falling back to 1 when no weight is provided; reference the _matchers list, get_weight, and the RequestMatcher/HttpRequestMatcher/HttpRequestRegexMatcher types so the implementation aligns with both declarative and programmatic matchers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 628-629: Current loop blindly forwards computed weight into every
policy; add a pre-check to reject weights that can never be satisfied by
moving-window policies before sleeping/retrying. Specifically, when computing
weight via get_weight(request), if the policy is a MovingWindowCallRatePolicy
call a guard (e.g., policy.can_fit(weight) or policy.validate_weight(weight)) or
inspect the moving-window config (max tokens/limit) and immediately raise a
deterministic exception (ValueError or a dedicated ConfigError) when weight >
policy's maximum capacity instead of calling try_acquire; keep behavior for
FixedWindowCallRatePolicy unchanged (it already fails fast).
---
Nitpick comments:
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 281-295: The get_weight function currently special-cases
HttpRequestRegexMatcher; instead, ask matchers for their weight via the matcher
abstraction: update get_weight to, for each matcher in self._matchers, call
matcher(request) and then obtain the weight by first trying a dedicated
method/property on the matcher (e.g., call matcher.get_weight(request) if
present or read matcher.weight via getattr), falling back to 1 when no weight is
provided; reference the _matchers list, get_weight, and the
RequestMatcher/HttpRequestMatcher/HttpRequestRegexMatcher types so the
implementation aligns with both declarative and programmatic matchers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9af6f1e9-f670-4292-b0e2-c3f4710aa295
📒 Files selected for processing (2)
airbyte_cdk/sources/streams/call_rate.pyunit_tests/sources/streams/test_call_rate.py
🚧 Files skipped from review as they are similar to previous changes (1)
- unit_tests/sources/streams/test_call_rate.py
Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
4390-4397:⚠️ Potential issue | 🟠 MajorCould we pass the matcher interpolation parameters into
weighthere, wdyt?Line 4393 still builds
InterpolatedStringwithparameters={}, so weights like{{ parameters.request_weight }}won't resolve even though string weights are supported. That leaves the new interpolation path broken for matcher/parent-parameter use cases.💡 Possible patch
weight = model.weight if weight is not None: + interpolation_parameters = model.parameters or kwargs.get("parameters", {}) or {} if isinstance(weight, str): - weight = int(InterpolatedString.create(weight, parameters={}).eval(config)) + weight = int( + InterpolatedString.create( + weight, + parameters=interpolation_parameters, + ).eval(config=config) + ) else: weight = int(weight)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py` around lines 4390 - 4397, The string-weight branch currently builds InterpolatedString with parameters={} so expressions like "{{ parameters.request_weight }}" won't resolve; change the InterpolatedString.create call for model.weight to pass the actual interpolation parameters (e.g., parameters) instead of an empty dict and then eval against config as before — update the InterpolatedString.create(...) call that constructs weight to use parameters=parameters (or the correct surrounding parameter variable) so matcher/parent-parameter weights interpolate correctly.
🧹 Nitpick comments (1)
airbyte_cdk/sources/streams/call_rate.py (1)
633-634: Could we resolveweightonce before the retry loop, wdyt?Line 633 recomputes the matcher-derived weight on every retry. Since the request and selected policy are fixed for a single
_do_acquire()call, moving this lookup above the loop would avoid repeated matcher scans on a hot path.💡 Possible patch
last_exception = None endpoint = self._extract_endpoint(request) + weight = policy.get_weight(request) if isinstance(policy, BaseCallRatePolicy) else 1 # sometimes we spend all budget before a second attempt, so we have a few more attempts for attempt in range(1, self._maximum_attempts_to_acquire): try: - weight = policy.get_weight(request) if isinstance(policy, BaseCallRatePolicy) else 1 policy.try_acquire(request, weight=weight)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airbyte_cdk/sources/streams/call_rate.py` around lines 633 - 634, In _do_acquire (call_rate.py) compute the weight once before entering the retry loop instead of inside the loop: determine weight = policy.get_weight(request) if isinstance(policy, BaseCallRatePolicy) else 1 prior to retrying, then call policy.try_acquire(request, weight=weight) on each attempt; this avoids repeated matcher scans while keeping the same policy and request semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py`:
- Around line 4390-4397: The string-weight branch currently builds
InterpolatedString with parameters={} so expressions like "{{
parameters.request_weight }}" won't resolve; change the
InterpolatedString.create call for model.weight to pass the actual interpolation
parameters (e.g., parameters) instead of an empty dict and then eval against
config as before — update the InterpolatedString.create(...) call that
constructs weight to use parameters=parameters (or the correct surrounding
parameter variable) so matcher/parent-parameter weights interpolate correctly.
---
Nitpick comments:
In `@airbyte_cdk/sources/streams/call_rate.py`:
- Around line 633-634: In _do_acquire (call_rate.py) compute the weight once
before entering the retry loop instead of inside the loop: determine weight =
policy.get_weight(request) if isinstance(policy, BaseCallRatePolicy) else 1
prior to retrying, then call policy.try_acquire(request, weight=weight) on each
attempt; this avoids repeated matcher scans while keeping the same policy and
request semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8aa79a64-0c99-4294-941e-d749bbce9ff6
📒 Files selected for processing (3)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.pyairbyte_cdk/sources/streams/call_rate.pyunit_tests/sources/streams/test_call_rate.py
🚧 Files skipped from review as they are similar to previous changes (1)
- unit_tests/sources/streams/test_call_rate.py
…Policy Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
Anatolii Yatsuk (tolik0)
approved these changes
Mar 27, 2026
Daryna Ishchenko (darynaishchenko)
merged commit 007066b
into
main
27 of 28 checks passed
Daryna Ishchenko (darynaishchenko)
deleted the
devin/1774609130-cost-based-rate-limiting
branch
1 task
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
2 participants
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
Adds an optional
weightfield toHttpRequestRegexMatcherthat allows specifying per-request weight in rate limiting policies. This enables weight-based rate limiting where different endpoints consume different amounts from a shared budget.Motivation: APIs like Amplitude's Dashboard REST API use a cost model where different endpoints have different costs (e.g.,
/2/sessions/average= 60 cost/req,/2/users= 120 cost/req) sharing a single budget (108,000 cost/hour). Previously,APIBudget._do_acquire()hardcodedweight=1even though the underlyingpyrate_limiteralready supports custom weights viatry_acquire(weight=...). This change wires up the existing weight infrastructure.Changes across 5 files (~170 lines):
weightfield (int or interpolated string) onHttpRequestRegexMatcher, defaults toNonecall_rate.py: Stores weight on matcher with validation (weight >= 1), addsBaseCallRatePolicy.get_weight()to look up weight from first matching matcher (short-circuiting consistently withmatches()), updatesAPIBudget._do_acquire()to pass dynamic weight totry_acquire(), adds weight guard toMovingWindowCallRatePolicy(rejects weights exceeding the lowest configured rate limit, matchingFixedWindowCallRatePolicy's existing guard)model_to_component_factory.py: Wires up weight field withInterpolatedStringsupport for config-driven values; validatesweight >= 1after interpolation/castingFully backward compatible —
weightdefaults toNone(treated as weight=1).Review & Testing Checklist for Human
get_weight()re-evaluates matchers in_do_acquire— Afterpolicy.matches(request)already confirmed a match,get_weight()re-iterates matchers. It short-circuits at the first match (consistent withmatches()), but verify the double-evaluation doesn't cause issues with stateful matchers or edge cases where the two methods could disagree on which matcher matched.get_weight()only checksHttpRequestRegexMatcher— Theisinstancecheck meansHttpRequestMatcher(non-regex variant) and customRequestMatcherimplementations will always get weight=1. Verify this is the intended scope.InterpolatedStringusesparameters={}— The factory passes an empty parameters dict, so{{ parameters.X }}won't resolve. Verify{{ config.X }}is sufficient for expected use cases.create_http_request_matcherpath with a string weight value (e.g.,"{{ config['weight'] }}"). Consider whether this gap matters for confidence.Notes
weightfield accepts strings to support Jinja interpolation from connector config, though the primary use case is static integer values.weight(notcost) for consistency with the existingtry_acquire(weight=...)parameter in the CDK's rate limiter API.>= 1) is enforced in bothHttpRequestRegexMatcher.__init__andcreate_http_request_matcherfactory (catches invalid manifest config after interpolation/casting).FixedWindowCallRatePolicyandMovingWindowCallRatePolicynow reject weights that exceed their configured limits with a deterministicValueError, preventing infinite retry loops.Summary by CodeRabbit
Link to Devin session: https://app.devin.ai/sessions/642ec5f275684572aa83aa9c7da87444
Requested by: Daryna Ishchenko (@darynaishchenko)