add the mixer feature flags in Dockerfile and use the custom.yaml in DCP#6327
Conversation
… DCP, when spanner embedding is set to true, the custom.yaml is updated to read from Spanner embeddings
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 enhances the mixer service deployment by integrating custom feature flag management. It ensures that the necessary configuration files are available within the container and provides a mechanism to programmatically tune feature flags at runtime to support spanner-based embedding resolution. 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 the 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 counterproductive. 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 configures the CDC services to use custom feature flags by copying the deploy/featureflags directory in the Dockerfile and updating run.sh to pass the feature flags path to the mixer. Additionally, it adds shell logic to enable Spanner search embeddings in the custom configuration if a specific environment variable is set. The review feedback points out a critical issue where the Docker build will fail because the deploy/featureflags directory is copied from a stage where it does not exist, and notes an inconsistency between the environment variable checked in the script and the one mentioned in the pull request description.
| # 3. Use mixer custom feature flags | ||
| MIXER_ARGS+=('--feature_flags_path=deploy/featureflags/custom.yaml') | ||
|
|
||
| if [[ $RESOLVE_WITH_SPANNER_EMBEDDINGS == "true" ]]; then |
There was a problem hiding this comment.
Why do you need this?
For website we needed it because the feature_flags file is also used by the CDC stable image, so we couldnt just override that file.
The custom.yaml you added is only for DCP. So is there a reason why you can't just set the variables in the custom.yaml?
FYI this override logic for website feature flags is meant to be deleted when we stop building the CDC stable image.
…th into the INFO log instead of directly into the high level debug log
This PR copy the feature flag folder in custom DC Dockerfile
It also add logic to include feature_flags_path for DCP only case, it will read the feature flag when use spanner graph.
We also add the logic to read an env variable RESOLVE_WITH_SPANNER_EMBEDDINGS to tune the yaml to do resolve through spanner embeddings