-
Notifications
You must be signed in to change notification settings - Fork 48
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: add Flipt provider #461
Conversation
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
@markphelps build passed locally but failing here since flipt-java is built with Java 11, and this job is build with Java 8. |
Signed-off-by: liran2000 <liran2000@gmail.com>
@liran2000 yeah I will try to setup a Java 8 release. Our Java SDK is actually autogenerated, so hopefully there arent any Java 9+ specific features (been a long time since I've written Java) |
Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
@liran2000 I dont think it's going to work with Java 8: https://github.com/flipt-io/flipt-java/actions/runs/6355901633/job/17264700354 Looks like our SDK requires some functionality that isn't in 8 (Map.of) |
is it possible to update this job to a more recent version of Java? |
I can try to take a look at Flipt SDK to see what is the amount of non compliant code.
As a library, if it compiles with newer Java version it will break Java 8 compatability, and will require newer Java version from all usages. |
Lots of enterprises are (unfortunately) still using Java 1.8, which is why both the SDK and contribs target (and use) it. @markphelps I suspect the biggest challenge will be getting your tooling/plugins/etc working with 8, though that's not strictly necessary. You should be able to target 1.8 and then you'll be only making code changes, which from 11 to 8 shouldn't be too huge 😬 ... but obviously it depends on your 11 language feature usage. |
@markphelps opened PR for Flipt SDK. |
Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
Hi @liran2000 ! We just released a new version which uses the sourceCompatibility / targetCompatibility features of gradle. Ive been told that this should be enough so that the JAR/class files should target Java 8. Would it be possible to update the Flipt SDK to 0.2.13? https://central.sonatype.com/artifact/io.flipt/flipt-java ? cc @dsinghvi |
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Thanks for the effort, the output JAR classes are now "Java 8", I tried it, it failed since the source code still have Java 8+ code. Failure is in the test, which is like a runtime test:
In the link provided, it is mentioned:
I don't see how it can compile and run with Java 8 without removing the incompatible Map.of from source code. |
@liran2000 thanks for testing -- this was oversight on my part. Will rerelease a version of the generator today. |
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.
looks great once the Flipt Java SDK is updated! thank you @liran2000 !!
Co-authored-by: Mark Phelps <209477+markphelps@users.noreply.github.com> Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
providers/flipt/src/main/java/dev/openfeature/contrib/providers/flipt/FliptProvider.java
Outdated
Show resolved
Hide resolved
providers/flipt/src/main/java/dev/openfeature/contrib/providers/flipt/FliptProvider.java
Show resolved
Hide resolved
providers/flipt/src/main/java/dev/openfeature/contrib/providers/flipt/FliptProvider.java
Outdated
Show resolved
Hide resolved
providers/flipt/src/main/java/dev/openfeature/contrib/providers/flipt/FliptProvider.java
Outdated
Show resolved
Hide resolved
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
I'm ready to approve once the above unresolved comments are resolved. |
Comments are addressed in recents commits, can you check latest code? |
Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
Oh I'm sorry, I kept following sent to old permalinks to previous commits! Yes I see they are resolved. Approving! |
Thank you @liran2000 for all your hard work on this!!! And thank you @toddbaert for the review. Will update our docs as well to mention this new provider |
…n-feature#461) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
add Flipt provider.
See readme for details.
@markphelps you are welcome to review from Flipt perspective.
@toddbaert you are welcome to review from OpenFeature perspective.