refactor: introduce add, update and delete operations to sync providers #307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kavindu-Dodan
requested review from
AlexsJones,
james-milligan and
toddbaert
as code owners
Kavindu-Dodan
force-pushed
the
refactor/sync-merge-strategy
branch
from
142a743
to
129d100
Compare
beeme1mr
reviewed
Jan 23, 2023
toddbaert
reviewed
Jan 24, 2023
Kavindu-Dodan
requested review from
toddbaert and
beeme1mr
and removed request for
thomaspoignant,
skyerus,
toddbaert and
beeme1mr
toddbaert
approved these changes
Jan 24, 2023
There was a problem hiding this comment.
Though I'm not sure we will anytime soon implement a sync that would take advantage of this... we might need one at some point to avoid large payloads for only small flag changes.
I think it's worth merging this to be able to easily add that support, but I would like @skyerus, @james-milligan or @AlexsJones 's opinion.
skyerus
reviewed
Jan 25, 2023
Kavindu-Dodan
requested review from
james-milligan and
toddbaert
and removed request for
beeme1mr and
james-milligan
Kavindu-Dodan
force-pushed
the
refactor/sync-merge-strategy
branch
2 times, most recently
from
99a7976
to
5648dc4
Compare
james-milligan
approved these changes
Jan 30, 2023
toddbaert
approved these changes
Jan 30, 2023
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Kavindu-Dodan
force-pushed
the
refactor/sync-merge-strategy
branch
from
2e620a8
to
722db95
Compare
beeme1mr
approved these changes
Jan 30, 2023
beeme1mr
pushed a commit
that referenced
this pull request
Feb 7, 2023
@Kavindu-Dodan has contributed multiple significant changes and proposals to flagd: - multiple refactors: #291, #307 - ci/security improvements: #338, #337 - architectural proposals (some of which got some attention from outside parties!): open-feature/ofep#45, open-feature/flagd-schemas#78, #249 (comment) - load testing: #225 - documentation improvements For these reasons, I believe he should be made a CODEOWNER in this repository. NOTE: before this is merged, @Kavindu-Dodan should be added with at least `maintainer` permissions to the repo. Signed-off-by: Todd Baert <toddbaert@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR
Introduce and wire new strategies to sync provider state updates.
Following new mechanisms were added to be used by sync providers (ex:- file based flag configuration provider),
These operations provide extra flexibility compared to the existing "merge" operation, which is now mapped to "ALL" (i.e - Add and replace all).
Test coverage of 100% for newly added methods.
NOTE - recommends commit by commit review to minimize confusion
Related Issues
Related to #249