-
Notifications
You must be signed in to change notification settings - Fork 50
feat(flags): Add ETag support for local evaluation polling #381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add support for HTTP conditional requests using ETags to reduce bandwidth when polling for feature flag definitions. When flag definitions haven't changed, the server returns 304 Not Modified and the SDK skips processing. - Add GetResponse dataclass to encapsulate response data, ETag, and status - Update get() to send If-None-Match header and handle 304 responses - Store ETag in client and pass it on subsequent polling requests - Skip flag processing when 304 Not Modified is received
Benefits: 1. Reuses TCP connections via keep-alive 2. 2 retries on connect/read errors 3. Faster handshakes
Test HTTP-level behavior including: - ETag extraction from response headers - If-None-Match header sent when etag provided - 304 Not Modified response handling - Fallback when 304 has no ETag header - Error response handling (APIError) - Authorization and User-Agent headers - Timeout and URL construction
Guard against unexpected None data in non-304 responses to prevent TypeError when accessing dictionary keys.
If the server stops including ETag headers in responses, clear the stored ETag so we don't keep sending a stale If-None-Match header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds HTTP conditional request support using ETags to optimize bandwidth usage when polling for feature flag definitions. When flags haven't changed, the server returns 304 Not Modified and the SDK skips reprocessing unchanged data.
Key Changes:
- Introduced
GetResponsedataclass to encapsulate HTTP response data, ETag headers, and modification status - Enhanced
get()function to sendIf-None-Matchheaders and handle 304 responses - Updated client to store and reuse ETags across polling requests
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
posthog/request.py |
Added GetResponse dataclass; updated get() to support ETag headers and 304 responses; modified remote_config() to extract data from GetResponse |
posthog/client.py |
Added _flags_etag field to store ETag; updated _load_feature_flags() to pass ETag on requests and skip processing on 304 responses |
posthog/test/test_request.py |
Added comprehensive unit tests for get() function's ETag handling and 304 response behavior |
posthog/test/test_feature_flags.py |
Updated mocks to return GetResponse objects; added tests for ETag storage, propagation, and 304 handling |
posthog/test/test_client.py |
Updated mocks to return GetResponse objects for consistency with API changes |
test_etag.py |
Added manual test script to observe ETag support in action during local evaluation polling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0349f7d to
1b57c72
Compare
Keep first 10 chars visible for identification while hiding the rest. Addresses CodeQL security warning about logging sensitive data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dustinbyrne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, no comments
Add support for HTTP conditional requests using ETags to reduce bandwidth when polling for feature flag definitions. When flag definitions haven't changed, the server returns 304 Not Modified and the SDK skips processing.
GetResponsedataclass to encapsulate response data, ETag, and statusget()to sendIf-None-Matchheader and handle 304 responsesETagin client and pass it on subsequent polling requests_sessioninstead ofrequestsforgetrequests (seems this was an existing oversight).