[Feature Flags] fetch request from context in is_feature_enabled()#6157
[Feature Flags] fetch request from context in is_feature_enabled()#6157juliawu merged 1 commit intodatacommonsorg:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the server-side feature flag utility by enabling automatic request resolution from the current Flask context. This change simplifies function signatures across the codebase, removing the requirement to manually pass request objects through complex call hierarchies while maintaining support for query parameter-based feature overrides. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the feature flag system to automatically retrieve the Flask request from the context if it is not explicitly provided to the is_feature_enabled function. Feedback suggests moving this logic into the underlying helper functions, is_feature_override_enabled and is_feature_override_disabled, to ensure consistent behavior and API usability across all public functions that accept an optional request argument.
| if request is None and has_request_context(): | ||
| request = flask_request |
There was a problem hiding this comment.
The logic to automatically fetch the request from the Flask context is a good improvement. However, for better consistency and API usability, this logic should ideally be moved into the helper functions is_feature_override_enabled and is_feature_override_disabled (or a shared internal helper). Since those functions are public and also take an optional request argument, updating them would ensure that the 'no-piping' benefit applies regardless of whether a developer calls the main is_feature_enabled function or the specific override helpers directly.
2 participants
This PR updates the
is_feature_enabled()server-side feature flag util to attempt to fetch the request from context if one is not explicitly provided.This will allow us to get
enable_feature=query parameter values from requests without needing to pipe in the request object through deep stacks of calls.Note that the flask request context is for the route being called. On the client side, when making flask API calls, we would just need to ensure
enable_feature=is included in the request to the flask APIs, even if the page calling the APIs already has the flag enabled. For example, even if we are ondatacommons.org?enable_feature=use_v2_api, if that page makes a call toapi/route, we would need to update the client side code to make a call toapi/route?enable_feature=use_v2_api.