Skip to content
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

fix: Increment flag & segment versions when reloading from file data source #285

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

keelerm84
Copy link
Member

The file data source allows specifying flag information as a full flag
definition, or as a shorted map of flag key:value mappings.

In the case of the flag values, or in the case of a malformed flag
definition, a flag version might not be specified. When this happens,
users of the flag tracker will notice an error because the version
comparison code will encounter an unexpected nil value.

To prevent this from happening, the file data source should be setting a
version for each flag or segment it reads.

When these items are modified in the LaunchDarkly UI, we automatically
increment the version associated with the item. To make this easier for
the user going forward, the file data source will handle incrementing
this version number each time the file is re-read.

…source

The file data source allows specifying flag information as a full flag
definition, or as a shorted map of flag key:value mappings.

In the case of the flag values, or in the case of a malformed flag
definition, a flag version might not be specified. When this happens,
users of the flag tracker will notice an error because the version
comparison code will encounter an unexpected nil value.

To prevent this from happening, the file data source should be setting a
version for each flag or segment it reads.

When these items are modified in the LaunchDarkly UI, we automatically
increment the version associated with the item. To make this easier for
the user going forward, the file data source will handle incrementing
this version number each time the file is re-read.
@keelerm84 keelerm84 requested a review from a team as a code owner June 6, 2024 16:27
@@ -40,6 +40,9 @@ def initialize(data_store, data_source_update_sink, logger, options={})
@poll_interval = options[:poll_interval] || 1
@initialized = Concurrent::AtomicBoolean.new(false)
@ready = Concurrent::Event.new

@version_lock = Mutex.new
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does result in some behavior change.

If you modify a single flag, all of the flags are going to appear to have changed since we are incrementing all of their versions. This is what the Java SDK does.

Copy link
Member

@kinyoklion kinyoklion Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably fine, but I know we have a few different ways this behaves.

Node: Only update the version if it thinks it changed, but the logic may not be the best. Each flag has its own version which increments from wherever it started if it was in the file data.
Dotnet: One shared version number that applies to all flags.

And some that had this previous behavior. Something we may want to write a spec for at some point.

Both are technically things that could happen with a real client.

@keelerm84 keelerm84 merged commit 7d5b051 into main Jun 10, 2024
6 checks passed
@keelerm84 keelerm84 deleted the mk/sc-245560/file-data branch June 10, 2024 17:04
keelerm84 pushed a commit that referenced this pull request Jun 10, 2024
🤖 I have created a release *beep* *boop*
---


##
[8.5.0](8.4.2...8.5.0)
(2024-06-10)


### Features

* Support to_h and to_json methods for LDContext
([#284](#284))
([d3c8d40](d3c8d40))


### Bug Fixes

* Increment flag & segment versions when reloading from file data source
([#285](#285))
([7d5b051](7d5b051))
* Log warning if client init timeout is considered high
([#278](#278))
([61f4c7e](61f4c7e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants