fix(flagd): use FLAGD_SYNC_PORT for in-process provider with backwards compatibility#345
fix(flagd): use FLAGD_SYNC_PORT for in-process provider with backwards compatibility#345MridulTailor wants to merge 2 commits intoopen-feature:mainfrom
Conversation
…s compatibility - Add ENV_VAR_SYNC_PORT constant for FLAGD_SYNC_PORT - Update port resolution: FLAGD_SYNC_PORT → FLAGD_PORT → default for in-process mode - Keep RPC mode using FLAGD_PORT only - Add 5 new tests covering sync port usage, fallback, priority, and RPC isolation - Update README documentation to reflect new environment variable Fixes open-feature#318 Signed-off-by: Mridul Tailor <mtailor729@gmail.com>
Summary of ChangesHello @MridulTailor, 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 refines the port configuration mechanism for the flagd Python provider, specifically enhancing the in-process resolver. It introduces Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 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 in-process provider to use FLAGD_SYNC_PORT for port configuration, with backwards compatibility for FLAGD_PORT. The changes are logical and well-tested. I've provided a couple of suggestions to improve code readability and documentation formatting.
| if self.resolver is ResolverType.IN_PROCESS: | ||
| # For in-process: try FLAGD_SYNC_PORT first, then FLAGD_PORT (backwards compatibility), then default | ||
| sync_port_env = os.environ.get(ENV_VAR_SYNC_PORT) | ||
| if sync_port_env is not None: | ||
| self.port = int(sync_port_env) | ||
| else: | ||
| port_env = os.environ.get(ENV_VAR_PORT) | ||
| self.port = ( | ||
| int(port_env) | ||
| if port_env is not None | ||
| else DEFAULT_PORT_IN_PROCESS | ||
| ) |
There was a problem hiding this comment.
This logic for determining the port for the in-process resolver can be simplified. Using os.environ.get(...) or os.environ.get(...) can make the code more concise and easier to read by reducing nesting.
if self.resolver is ResolverType.IN_PROCESS:
# For in-process: try FLAGD_SYNC_PORT, then FLAGD_PORT (backwards compatibility), then default
port_from_env = os.environ.get(ENV_VAR_SYNC_PORT) or os.environ.get(
ENV_VAR_PORT
)
self.port = (
int(port_from_env)
if port_from_env is not None
else DEFAULT_PORT_IN_PROCESS
)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
- Coverage 96.55% 95.55% -1.00%
==========================================
Files 5 16 +11
Lines 174 878 +704
==========================================
+ Hits 168 839 +671
- Misses 6 39 +33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Mridul Tailor <71081929+MridulTailor@users.noreply.github.com>
3 participants
Fixes #318
This PR
Updates the in-process provider to use
FLAGD_SYNC_PORTas the primary environment variable for port configuration, while maintaining backwards compatibility withFLAGD_PORTas a fallback. This aligns the Python SDK with the flagd specification and other OpenFeature SDKs.Changes made:
ENV_VAR_SYNC_PORT = "FLAGD_SYNC_PORT"constant toconfig.pyConfig.__init__:FLAGD_SYNC_PORT→FLAGD_PORT(fallback) →DEFAULT_PORT_IN_PROCESS(8015)FLAGD_PORT→DEFAULT_PORT_RPC(8013) (unchanged)portparameter always takes precedencetest_config.py:test_in_process_uses_sync_port- VerifiesFLAGD_SYNC_PORTis used when settest_in_process_fallback_to_port- VerifiesFLAGD_PORTfallback for backwards compatibilitytest_in_process_sync_port_priority- VerifiesFLAGD_SYNC_PORTtakes priority when both are settest_rpc_ignores_sync_port- Verifies RPC mode ignoresFLAGD_SYNC_PORTtest_in_process_default_port_when_no_env_var- Verifies default port when no env vars are setREADME.mdconfiguration table to document the new environment variableRelated Issues
Fixes #318
Parent issue: open-feature/flagd#1573
Testbed issue: open-feature/flagd-testbed#266
Notes
Backwards Compatibility:
FLAGD_PORTfor in-process mode continues to workFLAGD_SYNC_PORTfor in-process mode (recommended)How to test
Run the unit tests: