-
Notifications
You must be signed in to change notification settings - Fork 53
feat: Expose get_feature_flag_result method in public API
#284
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
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
Exposes get_feature_flag_result method in the PostHog Python client's public API, providing a structured way to access feature flag data.
- Added type-safe
get_feature_flag_resultmethod returning aFeatureFlagResultobject withenabled,variant, and auto-deserializedpayloadproperties - Method implementation follows existing patterns using
_proxyfor client communication - Added clear documentation and examples in both
__init__.pyandexample.pyshowing usage of the new method - Addresses JSON deserialization issues from #226 while maintaining backward compatibility
2 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
Adds a new public API method to fetch a fully detailed feature flag result and demonstrates its usage in the example script.
- Exposes
get_feature_flag_resultinposthog/__init__.py - Documents return object fields in the new method’s docstring
- Shows basic usage in
example.py
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| posthog/init.py | Added get_feature_flag_result wrapper and docstring |
| example.py | Added sample code calling the new get_feature_flag_result |
Comments suppressed due to low confidence (4)
posthog/init.py:356
- [nitpick] For consistency with
get_feature_flag_payload, consider adding an optionalmatch_valueparameter (and passing it through_proxy) if the underlying client supports it, or document why it’s intentionally omitted.
key,
posthog/init.py:355
- This new public API method should have corresponding unit tests to verify behavior under various flag states (enabled/disabled, with/without variants/payload).
def get_feature_flag_result(
example.py:64
- [nitpick] The example prints
enabled,variant, andpayloadbut omitskeyandreason, which are part ofFeatureFlagResult. Consider showing all fields to fully demonstrate the API.
result = posthog.get_feature_flag_result("beta-feature", "distinct_id")
posthog/init.py:355
- The return type hint
FeatureFlagResultis referenced but not imported in this module. Please add the appropriate import (e.g.from posthog import FeatureFlagResult) so the annotation resolves correctly.
def get_feature_flag_result(
…hods
- Changed default parameters from {} to None for all feature flag methods
- Prevents potential bugs from shared mutable defaults across function calls
- Fully backwards compatible - callers see no change in behavior
- Follows Python best practices for function parameter defaults
Also:
- Removed exit() call from example.py that was preventing remaining examples from running
- Expanded get_feature_flag_result example to demonstrate all available fields
Addresses review feedback from PR #284
e83a661 to
9112a73
Compare
dmarticus
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.
this seems reasonable; but didn't we already ship get_feature_flag_result in posthog-python 4.0.0? How is this significantly different?
Oh geez. I was working with outdated info. However, I think my changes to replace default The issue with mutable default parameters (like {} or []) is that they are created once when the function is defined, not each time the function is called. This can lead to unexpected behavior where the default object is shared across all function calls. In our code, this bug likely wouldn't manifest in practice (since the parameters are passed through to other functions rather than being modified), it's still a code smell. I can fix this PR to solely address that. |
|
Oh wait a minute. We did add |
|
@haacked is this still relevant? Don't we already expose this method? |
It is still revelant. This PR exports |
This exposes the existing get_feature_flag_result method that returns a FeatureFlagResult object containing enabled, variant, and payload properties. The payload is automatically deserialized from JSON. Fixes #226
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…hods
- Changed default parameters from {} to None for all feature flag methods
- Prevents potential bugs from shared mutable defaults across function calls
- Fully backwards compatible - callers see no change in behavior
- Follows Python best practices for function parameter defaults
Also:
- Removed exit() call from example.py that was preventing remaining examples from running
- Expanded get_feature_flag_result example to demonstrate all available fields
Addresses review feedback from PR #284
- Fixed mutable default parameters in all client.py methods - Added proper None handling throughout the codebase - Ensured type safety for mypy by adding assertions and proper handling - All feature flag methods now use None defaults and handle them correctly - Resolved all new mypy violations introduced by the changes This comprehensive fix prevents potential bugs from shared mutable defaults across all feature flag functionality in the PostHog Python client.
9112a73 to
f9f63df
Compare
This exposes the existing
get_feature_flag_resultmethod that returns aFeatureFlagResultobject containingenabled,variant, andpayloadproperties. The payload is automatically deserialized from JSON.Related to #226 where I suggested I would add this method. Here I go.