feat(feature-flags): support quota limiting for feature flags #195
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds support for handling quota-limited feature flag responses in the PostHog Python SDK, while also fixing case sensitivity issues in feature flag payload evaluation.
- Added
QuotaLimitErrorhandling inrequest.pyto properly handle when users exceed their feature flag quota limits - Modified
_process_responseto detect quota-limited responses from/decideand/local_evaluationAPIs with appropriate error logging - Added test coverage in
test_request.pyfor both quota-limited and normal response scenarios - Fixed case sensitivity issues in feature flag payload evaluation (issue Feature flag evaluation is case sensitive, while feature flag payload match evaluation is evaluated as lower() #178)
- Updated version to 3.14.0 to reflect these new features
6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
posthog/request.py
Outdated
| return res.json() if return_json else res | ||
| response = res.json() if return_json else res | ||
| # Handle quota limited decide responses by raising a specific error | ||
| if isinstance(response, dict) and "quotaLimited" in response and "feature_flags" in response["quotaLimited"]: |
There was a problem hiding this comment.
style: Consider checking for quotaLimited being a list before checking if feature_flags is in it to avoid potential type errors
dmarticus
changed the title
| self.feature_flags = [] | ||
| self.feature_flags_by_key = {} | ||
| self.group_type_mapping = {} | ||
| self.cohorts = {} |
There was a problem hiding this comment.
nit: should we stay consistent with the 401 case above and raise an APIError if debug mode is on?
| isinstance(response, dict) | ||
| and "quotaLimited" in response | ||
| and isinstance(response["quotaLimited"], list) | ||
| and "feature_flags" in response["quotaLimited"] |
There was a problem hiding this comment.
nit: I'm assuming this is to cover the /decide case, correct? Can / should we avoid the special condition for feature_flags and instead log any quota limited services, maybe raising an error for any of them (or, keep a list of services that should throw an error if quota limited)
There was a problem hiding this comment.
oh yeah, good call. I think for now I explicitly want to limit feature flag events for now (since I don't know how other services are handling quota limits in the SDK).
There was a problem hiding this comment.
I'll make a comment to that end though.
with this PR, we now start to respond different with the
/decideand/local_evaluationAPIs if users have gone over their quota limit. This change is an example for how our SDKs can deal with that. I'll make the same type of changes to all of the SDKs, but wanted to start with this one as an example.