-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: ISSUE-658 go-feature-flag sdk - add cache #369
feat: ISSUE-658 go-feature-flag sdk - add cache #369
Conversation
…data collector events Signed-off-by: liran2000 <[email protected]>
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.
@liran2000 thanks a lot for your contribution, this is really awesome, and I can't wait to have the cache in the JAVA provider 🙌.
I've done a first review pass and found some points that can be improved, let me know if you need any further information about them.
...ature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/BeanUtils.java
Outdated
Show resolved
Hide resolved
...ature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/BeanUtils.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
.../main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderOptions.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
Signed-off-by: liran2000 <[email protected]>
- added events publish retry mechanism support - added shutdown Signed-off-by: liran2000 <[email protected]>
…b/providers/gofeatureflag/bean/BeanUtils.java Signed-off-by: Thomas Poignant <[email protected]>
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.
Hey, I've spotted some minor things here and there to fix some warnings.
The main thing we should tackle is that we should probably override the method initialize
to initialize the collection in the background and to use fully the SDK capabilities.
For this what we may do is move the code from initializeProvider
to initialize
and remove the call from the constructor.
If we move the code there we will also have to implement the getStatus()
method and have the provider status lifecycle ready in the provider.
I've done something similar in the node provider here if you want to have a look: open-feature/js-sdk-contrib#493
If you prefer, I'll be happy to help you with that myself and to collaborate on this pull request, but this up to you.
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
.../main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderOptions.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
Signed-off-by: liran2000 <[email protected]>
…nto go-feature-flag-provider-658d
…nitialized Signed-off-by: liran2000 <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
8923a47
to
a9d27b5
Compare
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.
@thomaspoignant thanks for reviewing, please see my comments on the commit, as it affects the flow.
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...lag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
Outdated
Show resolved
Hide resolved
...src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java
Show resolved
Hide resolved
Signed-off-by: liran2000 <[email protected]>
Signed-off-by: liran2000 <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
76a2954
to
dc9ed21
Compare
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
af504c0
to
f02fd35
Compare
This PR
Implementing issue 658: (feature) Implement open-feature provider cache for the Java provider.
Related Issues
issue 658
Notes
How to test