-
Notifications
You must be signed in to change notification settings - Fork 55
chore: Use FDv1 DataSystem in the ldclient #345
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
Conversation
| # @return [Array<EvaluationDetail, [LaunchDarkly::Impl::Model::FeatureFlag, nil], [String, nil]>] | ||
| # | ||
| def variation_with_flag(key, context, default) | ||
| private def variation_with_flag(key, context, default) |
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.
The file used a mix of per method private and all methods after the declaration on line 717 of the original file. Moving to defining per method.
| # | ||
| def initialized? | ||
| @config.offline? || @config.use_ldd? || @data_source.initialized? | ||
| @data_system.data_availability == @data_system.target_availability |
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.
Are we sure these lines are equivalent given our previous discussions about this? I can't remember exactly where we landed on that.
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.
They should be equivalent for FDv1. This will need to be altered a bit when we introduce FDv2.
- Offline returns and expects
DEFAULTS✅ - Use_Ldd returns and expects
REFRESHED(because it uses null processor) ✅ - If the data_source.Initialized is true the data_system returns
REFRESHEDandCACHEDif false, but we expectREFRESHED✅
Note
Adopt FDv1 data system in client
Impl::DataSystem::FDv1; FDv1 wraps the originalfeature_storeonce (avoids nested wrappers), injectsdata_source_update_sink, exposesstore,flag_change_broadcaster, and status providers, and cleans up viastop(includingstore_wrapper.stop).data_store_status_provideranddata_source_status_providerfrom LDClient to the data system; addflushdelegator to the event processor; refine SDK key nil validation for offline/LDD/custom sources.initialized?now comparesdata_availabilityvstarget_availability; all reads switched todata_system.storein evaluations andall_flags_state.Tests
spec/impl/data_system/fdv1_spec.rbcovering store wrapping (no nested wrappers), processor selection (streaming/polling/offline/LDD), custom data source factory/instance handling, start/stop idempotency, availability semantics, diagnostic accumulator integration, and provider/broadcaster access.Written by Cursor Bugbot for commit 59ae3c0. This will update automatically on new commits. Configure here.