Add new feature flag for v2 API changes#6153
Conversation
juliawu
requested review from
SandeepTuniki,
n-h-diaz,
nick-nlb and
rohitkumarbhagat
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 establishes a new feature flag to gate upcoming frontend changes related to the v2 API migration. By centralizing this configuration across different environments, the team can safely roll out and test new API integrations without impacting production or stable environments. 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 introduces a new feature flag, use_v2_api, across multiple environment configuration files, enabling it for autopush, dev, and local environments while keeping it disabled for custom and staging. Feedback suggests refining the flag's description to clarify its focus on frontend migration and recommends defining a backend constant for the flag to ensure consistency. Additionally, the reviewer noted that the production configuration file is missing from the PR despite being referenced in the description.
| "name": "use_v2_api", | ||
| "enabled": true, | ||
| "owner": "juliawu", | ||
| "description": "Enables features that rely on the V2 REST APIs" |
There was a problem hiding this comment.
To maintain consistency with the existing feature flag implementation and improve clarity, please consider the following:
- Add a constant: Define a constant for this flag in server/lib/feature_flags.py (e.g., USE_V2_API_FEATURE_FLAG = 'use_v2_api') to avoid using hardcoded strings when checking the flag in the backend.
- Specific Description: The current description is quite broad. Since this flag is specifically for gating frontend changes during the v0/v1 migration, updating the description to reflect this will help avoid confusion with existing V2 API usage in the codebase.
| "name": "use_v2_api", | |
| "enabled": true, | |
| "owner": "juliawu", | |
| "description": "Enables features that rely on the V2 REST APIs" | |
| "name": "use_v2_api", | |
| "enabled": true, | |
| "owner": "juliawu", | |
| "description": "Enables frontend features migrating to V2 REST APIs" |
| "description": "Enables standardized visualization tool UI for the /tool/map, /tool/scatter, and /tool/timeline pages" | ||
| }, | ||
| { | ||
| "name": "use_v2_api", |
There was a problem hiding this comment.
The pull request description states that this flag should be disabled for the prod environment. However, server/config/feature_flag_configs/prod.json is not included in this PR.
While the feature flag system defaults to false if a key is missing, it is best practice to explicitly define the flag in all environment configurations for better visibility and maintainability.
| }, | ||
| { | ||
| "name": "use_v2_api", | ||
| "enabled": true, |
There was a problem hiding this comment.
The flags dont exist today, so they arent connected to anything.
I think you should set it all to False. Then put up the PR where you set the flag in the code, and then enable it where it needs to be enabled.
It ll be more explicit that way and we will be able to approve the enablements based on what the flag controls. Since we dont know exactly what the flag is going to control today, it makes it uncertain how the default should apply.
Note rolling out flags takes seconds :) More tiny PRs isnt a bad thing!
There was a problem hiding this comment.
Sounds good.
I switched everything to False, and we'll enable as we add features in subsequent PRs.
3 participants
Adds a new feature flag
use_v2_apito gate frontend changes for the v0/v1 migration.We'll enable the flag in subsequent PRs. Note that following the
create_feature_flags.shscript, we have not added the feature flag to the prod environment yet. We will add it in a separate PR when we are ready to deploy changes to prod.