-
Notifications
You must be signed in to change notification settings - Fork 54
chore: Add FDv2 compatible data source for testing #347
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
| until @closed | ||
| begin | ||
| # stop() will push nil to the queue to wake us up when shutting down | ||
| update = @update_queue.pop |
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.
@keelerm84 Ruby doesn't have a pop with timeout like we do in Python. The stop method will push nil into the queue to stop it from blocking so I'm not sure it is needed but I wanted to call it out.
| end | ||
|
|
||
| instances_copy.each do |instance| | ||
| instance.upsert_flag(new_flag) |
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.
Variable scoped to block but used outside
High Severity
The new_flag variable is defined inside the with_write_lock block on line 121, but it's used outside the block on line 131. In Ruby, variables defined inside a block are local to that block and not accessible outside. This will cause a NameError at runtime when update is called. The original test_data.rb correctly handles this by defining new_flag = nil before the block.
|
|
||
| private def add_rule(flag_rule_builder) | ||
| @_rules << flag_rule_builder | ||
| end |
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.
Private method called with explicit receiver fails
High Severity
The add_rule method is defined as private def add_rule(...), but it's called from FlagRuleBuilderV2#then_return with an explicit receiver (@_flag_builder.add_rule(self)). In Ruby, private methods cannot be called with an explicit receiver from a different object, which will cause a NoMethodError at runtime. The original flag_builder.rb correctly makes add_rule a public method (only marked with # @api private comment).
Additional Locations (1)
| # Add all flags to the changeset | ||
| init_data.each do |key, flag_data| | ||
| builder.add_put( | ||
| LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, |
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.
Should you be considering segments as well?
It looks like the old test data might have supported both flags and segments. Did we lose some functionality during this update?
| # consider it part of the public API, but it is still called from TestDataV2. | ||
| # | ||
| # Creates a deep copy of the flag builder. Subsequent updates to the | ||
| # original `FlagBuilderV2` object will not update the copy and vise versa. |
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.
| # original `FlagBuilderV2` object will not update the copy and vise versa. | |
| # original `FlagBuilderV2` object will not update the copy and vice versa. |
Note
Introduces an FDv2-compatible in-memory test data source with dynamic updates.
TestDataV2to manage test flags, provideflagbuilders,updatepropagation, versioning, and to build FDv2initializer/synchronizerTestDataSourceV2that fulfillsInitializerandSynchronizer, emitting initial fullChangeSetand incremental updates via a queue; supportsstopand error signalingFlagBuilderV2andFlagRuleBuilderV2to define variations, targets, and simple "in/not in" rules, producing FDv2-compatible flag hashesWritten by Cursor Bugbot for commit 5003b43. This will update automatically on new commits. Configure here.