-
Notifications
You must be signed in to change notification settings - Fork 23
feat(flagd): Implement dual-mode selector handling (header and body) for in-process mode in python-sdk-contrib #320
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
...der-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/connector/grpc_watcher.py
Outdated
Show resolved
Hide resolved
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.
Code Review
This pull request successfully implements dual-mode selector handling for the flagd provider's in-process mode, ensuring backward compatibility by passing the selector in both gRPC metadata and the request body. The changes are well-implemented across the provider logic, type definitions, and documentation. A new test case effectively verifies the dual-transmission behavior. My review includes a minor suggestion to improve code readability in the test suite. Overall, this is a solid contribution that enhances compatibility.
providers/openfeature-provider-flagd/tests/test_grpc_watcher.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Simon Schrottner <[email protected]>
- Add comprehensive migration guidance section for selector handling - Clarify that selector changes only affect in-process and file modes - Document backward compatibility and future breaking changes - Update provider docstring with selector migration note - Link to upstream flagd issue #1814 Co-authored-by: aepfli <[email protected]> Signed-off-by: Simon Schrottner <[email protected]>
- Update GrpcWatcher to pass selector via flagd-selector metadata header - Add GrpcMultiCallableArgs.metadata field to support gRPC metadata - Update README to reflect current header-based implementation - Add test to verify selector is passed via metadata, not request body - Aligns with flagd v0.11.0+ selector normalization standard Co-authored-by: aepfli <[email protected]> Signed-off-by: Simon Schrottner <[email protected]>
…lity - Update _create_request_args to include selector in request body - Maintain selector in flagd-selector gRPC metadata header - Update test to verify both header and body contain selector - Update documentation to reflect dual transmission approach - Ensures compatibility with both older and newer flagd versions Co-authored-by: aepfli <[email protected]> Signed-off-by: Simon Schrottner <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
5e905c7 to
75dc969
Compare
Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #320 +/- ##
==========================================
+ Coverage 94.56% 95.53% +0.96%
==========================================
Files 22 16 -6
Lines 1068 874 -194
==========================================
- Hits 1010 835 -175
+ Misses 58 39 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/gemini review |
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.
Code Review
This pull request successfully implements dual-mode selector handling for the flagd provider's in-process mode, ensuring backward compatibility by passing the selector in both the gRPC metadata header and the request body. The changes are well-structured, with logic correctly refactored into new methods in GrpcWatcher and types updated accordingly. A new test case has been added to verify the dual-transmission behavior. The documentation has also been updated to reflect these changes. I've found a minor inaccuracy in the README documentation that should be addressed.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>
This PR implements dual-mode selector passing for the flagd provider, specifically for in-process and file resolver modes only. RPC mode is not affected by these changes.
Context
Refer to open-feature/flagd#1814 and the related implementation change to selector normalization across flagd services.
Changes Made
Implementation Updates
GrpcWatcherto pass selector via bothflagd-selectorgRPC metadata header and request bodyGrpcMultiCallableArgsto support gRPC metadata headersDocumentation Updates
selectorparameter documentation to reflect dual transmission approachKey Implementation Details
flagd-selectorgRPC metadata header (for flagd v0.11.0+ selector normalization)Technical Changes
GrpcWatcher (
grpc_watcher.py):_create_metadata()method to construct gRPC metadata withflagd-selectorheader_create_request_args()to include selector in request body for backward compatibilitylisten()method to include metadata in gRPC callsType Definitions (
types.py):metadatafield toGrpcMultiCallableArgsTypedDictTest Suite (
test_grpc_watcher.py):test_selector_passed_via_both_metadata_and_body()to verify selector is passed via both methodsFixes #319
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.