Update py sdk exposure event tracking and add docs #144
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #144 +/- ##
==========================================
+ Coverage 92.45% 93.13% +0.68%
==========================================
Files 8 8
Lines 1153 1151 -2
Branches 76 76
==========================================
+ Hits 1066 1072 +6
+ Misses 56 49 -7
+ Partials 31 30 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
efahk
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a deadlock issue when tracking exposure events in the Python SDK by removing ThreadPoolExecutor usage and switching to direct synchronous calls for exposure event tracking. The change improves predictability across different Python runtimes, particularly when dealing with patched threading modules like eventlet.
- Removed ThreadPoolExecutor from both local and remote feature flags providers and switched to direct tracker calls
- Added comprehensive documentation to public methods in both providers
- Updated polling implementation to use a dedicated thread instead of executor submission
- Refactored test structure to use pytest fixtures for better test organization
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mixpanel/flags/types.py | Removed custom_executor field from FlagsConfig |
| mixpanel/flags/remote_feature_flags.py | Removed ThreadPoolExecutor usage, added method documentation, switched to direct tracker calls |
| mixpanel/flags/local_feature_flags.py | Replaced ThreadPoolExecutor with threading.Thread for polling, added comprehensive documentation, updated exposure tracking to direct calls |
| mixpanel/flags/test_remote_feature_flags.py | Removed executor shutdown calls from tests |
| mixpanel/flags/test_local_feature_flags.py | Refactored tests to use pytest fixtures and removed executor-related cleanup |
| mixpanel/init.py | Version bump to 5.0.0b2 |
| CHANGES.txt | Added changelog entry for the new version |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
3 participants
Submitting the call to track an exposure event on a threadpool was deadlocking in eventlet when threading module is patched.
For now, for more predictability across runtimes, updating calling track directly. Clients can override the consumer with more complex send behavior.
Other updates to use a single thread for polling, adding some public method comments, & some test refactors
GitHub Copilot Summary
This pull request updates the local feature flags evaluation logic and improves documentation, while also making several code quality and performance enhancements. The most significant change is the removal of the thread pool for exposure event tracking in local flags, which simplifies concurrency handling and improves reliability. Additional improvements include adding detailed docstrings to key methods, refactoring for clarity, and updating logging practices.
Local feature flags concurrency and exposure event tracking:
ThreadPoolExecutorfor exposure event tracking inLocalFeatureFlagsProvider, switching to direct function calls for better reliability and simplicity. This affects both the tracking logic and polling thread management. [1] [2]Documentation and code clarity:
local_feature_flags.pyandremote_feature_flags.py, making usage and intent clearer for future maintainers. [1] [2] [3]Logging and status reporting:
Version and changelog updates:
5.0.0b2and updatedCHANGES.txtto reflect the new changes. [1] [2]Remote feature flags cleanup:
remote_feature_flags.pyfor consistency with local flags changes. [1] [2]Let me know if you want to walk through any of the specific changes in more detail!