[v0/v1 migration] Gate v2/recognize/places behind feature flag#6156
[v0/v1 migration] Gate v2/recognize/places behind feature flag#6156juliawu merged 10 commits 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 introduces a feature flag mechanism to manage the transition between v1 and v2 API endpoints for place recognition. By gating the v2 endpoint behind the 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a feature flag to toggle between the v1 and v2 versions of the recognize places API. The review feedback identifies a missing constant definition that would cause an import error and recommends adding a request context check to prevent runtime errors when the code is executed outside of a Flask environment.
I am having trouble creating individual review comments. Click here to see my feedback.
server/services/datacommons.py (42)
The constant USE_V2_API is not defined in server/lib/feature_flags.py. This will cause an ImportError when the server attempts to start. Please ensure the constant is defined in the feature flags library (e.g., USE_V2_API = 'use_v2_api').
server/services/datacommons.py (731)
Accessing the request object directly here can lead to a RuntimeError if recognize_places is called outside of a Flask request context. Additionally, when using the v2observation API, ensure the 'value' field is included in the 'select' list, as it is required to return any data. Please update the request logic accordingly.
if is_feature_enabled(USE_V2_API, request=request if has_request_context() else None):
References
- The v2observation API requires the 'value' field to be present in the 'select' list to return any data, even if the 'value' field itself is not used.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature flag, USE_V2_API, to conditionally switch between V1 and V2 endpoints for the recognize_places service. Feedback identifies a potential RuntimeError if the code runs outside a Flask application context and a bug regarding query casing in the response, as V1 endpoints typically require lowercased keys while V2 does not. A code suggestion was provided to address these issues by adding context checks and version-specific key handling.
|
Quick note: the failing test here is just a python lint request via |
This PR puts the change made by Rohit in #6139 behind the
use_v2_apifeature flag.