feat: add shutdown API, provider status, and status short-circuit#223
feat: add shutdown API, provider status, and status short-circuit#223josecolella merged 3 commits intomainfrom
Conversation
…ircuit (spec 1.6, 1.7) - Add API#shutdown that propagates shutdown to all registered providers and clears state (spec 1.6.1, 1.6.2) - Add Client#provider_status accessor returning provider's current state from ProviderStateRegistry (spec 1.7.1) - Add short-circuit in Client#fetch_details for NOT_READY and FATAL provider states, returning default values with appropriate error codes (spec 1.7.6, 1.7.7) - Make Configuration#provider_state and #provider_tracked? public - Add ProviderStateRegistry#tracked? to check if a provider is registered (prevents false NOT_READY for directly-constructed clients) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
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 significantly enhances the OpenFeature SDK's compliance with the specification by introducing robust mechanisms for managing provider lifecycles and handling their operational states. It allows for graceful shutdown of providers, provides a clear way to query their readiness, and ensures that flag evaluations are handled safely and predictably when providers are not fully operational. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces several key features aligned with the OpenFeature specification, including a shutdown API for registered providers, a client-side accessor for provider status, and short-circuiting logic in flag evaluation for NOT_READY and FATAL provider states. The changes are well-tested and contribute significantly to the SDK's compliance and robustness.
lib/open_feature/sdk/client.rb
Outdated
| if OpenFeature::SDK.configuration.provider_tracked?(@provider) && state == ProviderState::NOT_READY | ||
| resolution = Provider::ResolutionDetails.new( | ||
| value: default_value, | ||
| error_code: Provider::ErrorCode::PROVIDER_NOT_READY, | ||
| reason: Provider::Reason::ERROR | ||
| ) | ||
| return EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution) | ||
| elsif OpenFeature::SDK.configuration.provider_tracked?(@provider) && state == ProviderState::FATAL | ||
| resolution = Provider::ResolutionDetails.new( | ||
| value: default_value, | ||
| error_code: Provider::ErrorCode::PROVIDER_FATAL, | ||
| reason: Provider::Reason::ERROR | ||
| ) | ||
| return EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution) |
There was a problem hiding this comment.
The logic for handling NOT_READY and FATAL provider states in fetch_details contains some duplication in the creation of Provider::ResolutionDetails and EvaluationDetails. This can be refactored to improve readability and reduce redundancy by determining the error_code once and then constructing the resolution details.
For example, you could set the error_code based on the state within a single conditional block.
if OpenFeature::SDK.configuration.provider_tracked?(@provider)
error_code = nil
case state
when ProviderState::NOT_READY
error_code = Provider::ErrorCode::PROVIDER_NOT_READY
when ProviderState::FATAL
error_code = Provider::ErrorCode::PROVIDER_FATAL
end
if error_code
resolution = Provider::ResolutionDetails.new(
value: default_value,
error_code: error_code,
reason: Provider::Reason::ERROR
)
return EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution)
end
endReplace copy-pasted NOT_READY/FATAL blocks with a single guard that uses a case statement helper method. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
Summary
API#shutdownthat propagates shutdown to all registered providers and clears state (spec 1.6.1, 1.6.2)Client#provider_statusaccessor returning the provider's current state (spec 1.7.1)Client#fetch_detailsforNOT_READYandFATALprovider states — returns default value with appropriate error code instead of calling the provider (spec 1.7.6, 1.7.7)ProviderStateRegistry#tracked?to distinguish unregistered providers from NOT_READY onesSpec References
Part of the spec compliance roadmap: #220
Test plan
bundle exec rspec spec/specification/flag_evaluation_api_spec.rb— all passbundle exec rspec spec/open_feature/sdk/client_spec.rb— all pass (no regression)bundle exec standardrb— no offenses🤖 Jose's AI agent