-
Notifications
You must be signed in to change notification settings - Fork 35
feat(flags): make the sendFeatureFlags parameter more declarative and ergonomic #283
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
base: master
Are you sure you want to change the base?
Conversation
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.
PR Summary
Enhanced the send_feature_flags
parameter in PostHog Python SDK to support a more declarative configuration object while maintaining backward compatibility with boolean values.
- Added new
SendFeatureFlagsOptions
TypedDict inposthog/types.py
allowing granular control over feature flag evaluation - Implemented support for event-specific person and group properties in
posthog/client.py
without affecting global state - Extended test coverage in
posthog/test/test_client.py
to verify new features like local evaluation and custom properties - Updated CHANGELOG.md and version.py to 6.2.0 following semantic versioning for the new feature
6 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
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
Enhance the send_feature_flags
parameter so it can accept a structured options object for explicit local vs. remote evaluation and per-event properties.
- Add
SendFeatureFlagsOptions
TypedDict and update argument types to support the new object - Refactor
Client.capture
to parse boolean or options object and branch into local or remote flag evaluation - Introduce tests covering
only_evaluate_locally=True
,False
, and default behaviors; bump version to 6.2.0 and update changelog
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
posthog/version.py | Bumped SDK version to 6.2.0 |
posthog/types.py | Added SendFeatureFlagsOptions TypedDict for granular flag control |
posthog/args.py | Updated OptionalCaptureArgs to allow boolean or options object |
posthog/client.py | Refactored capture to parse and apply send_feature_flags options |
posthog/test/test_client.py | Added tests for new send_feature_flags option behaviors |
CHANGELOG.md | Documented the enhanced send_feature_flags parameter |
Comments suppressed due to low confidence (3)
posthog/test/test_client.py:755
- [nitpick] Add a test case where
group_properties
is provided along withonly_evaluate_locally=True
to verify that local evaluation correctly applies the group-level overrides.
def test_capture_with_send_feature_flags_options_only_evaluate_locally_true(
posthog/args.py:37
- [nitpick] In the docstring for
send_feature_flags
, explicitly note that omittingonly_evaluate_locally
in the options object will default to remote evaluation so users clearly understand the fallback behavior.
send_feature_flags: NotRequired[
posthog/version.py:1
- Ensure that the stray hyphen from the previous version line is fully removed so the file only contains the clean
VERSION = "6.2.0"
statement.
VERSION = "6.2.0"
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.
Nice! Some stylistic suggestions and comment changes, but nothing blocking.
posthog/client.py
Outdated
only_evaluate_locally=True, | ||
) | ||
elif only_evaluate_locally is False: | ||
# Force remote evaluation via /decide |
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.
# Force remote evaluation via /decide | |
# Force remote evaluation via /flags |
posthog/client.py
Outdated
elif only_evaluate_locally is False: | ||
# Force remote evaluation via /decide | ||
feature_variants = self.get_feature_variants( | ||
distinct_id, | ||
groups, | ||
person_properties=flag_person_properties, | ||
group_properties=flag_group_properties, | ||
disable_geoip=disable_geoip, | ||
) | ||
else: | ||
# Default behavior - use remote evaluation | ||
feature_variants = self.get_feature_variants( | ||
distinct_id, | ||
groups, | ||
person_properties=flag_person_properties, | ||
group_properties=flag_group_properties, | ||
disable_geoip=disable_geoip, | ||
) |
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.
These two branches of logic do the same thing, so we can combine them, unless I'm missing something.
elif only_evaluate_locally is False: | |
# Force remote evaluation via /decide | |
feature_variants = self.get_feature_variants( | |
distinct_id, | |
groups, | |
person_properties=flag_person_properties, | |
group_properties=flag_group_properties, | |
disable_geoip=disable_geoip, | |
) | |
else: | |
# Default behavior - use remote evaluation | |
feature_variants = self.get_feature_variants( | |
distinct_id, | |
groups, | |
person_properties=flag_person_properties, | |
group_properties=flag_group_properties, | |
disable_geoip=disable_geoip, | |
) | |
else: | |
# only_evaluate_locally is False or None so use remote evaluation (Default behavior) | |
feature_variants = self.get_feature_variants( | |
distinct_id, | |
groups, | |
person_properties=flag_person_properties, | |
group_properties=flag_group_properties, | |
disable_geoip=disable_geoip, | |
) |
posthog/types.py
Outdated
Args: | ||
only_evaluate_locally: Whether to only use local evaluation for feature flags. | ||
If True, only flags that can be evaluated locally will be included. | ||
If False, remote evaluation via /decide API will be used when needed. |
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.
If False, remote evaluation via /decide API will be used when needed. | |
If False, remote evaluation via /flags API will be used when needed. |
Problem
The current
send_feature_flags
boolean parameter provides limited control over feature flag evaluation behavior. Users couldn't specify evaluation preferences or custom properties per capture event.Solution
Modified
send_feature_flags
to accept either:bool
(existing behavior, fully backward compatible)SendFeatureFlagsOptions
object with granular control:Benefits
Usage Examples
This brings the Python SDK in line with the enhanced Node.js SDK functionality from PostHog/posthog-js-lite#566