Skip to content

feat: add Flipt provider#461

Merged
toddbaert merged 20 commits into
open-feature:mainfrom
liran2000:feature/flipt
Oct 24, 2023
Merged

feat: add Flipt provider#461
toddbaert merged 20 commits into
open-feature:mainfrom
liran2000:feature/flipt

Conversation

@liran2000

Copy link
Copy Markdown
Member

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.

Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
@liran2000

Copy link
Copy Markdown
Member Author

@markphelps build passed locally but failing here since flipt-java is built with Java 11, and this job is build with Java 8.
Possibly flipt-java can have a Java 8 release, for example 0.2.11-java8 ?

Signed-off-by: liran2000 <liran2000@gmail.com>
@markphelps

Copy link
Copy Markdown
Contributor

@markphelps build passed locally but failing here since flipt-java is built with Java 11, and this job is build with Java 8. Possibly flipt-java can have a Java 8 release, for example 0.2.11-java8 ?

@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>
@markphelps

Copy link
Copy Markdown
Contributor

@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)

@markphelps

Copy link
Copy Markdown
Contributor

is it possible to update this job to a more recent version of Java?

@liran2000

Copy link
Copy Markdown
Member Author

Looks like our SDK requires some functionality that isn't in 8 (Map.of)

I can try to take a look at Flipt SDK to see what is the amount of non compliant code.

is it possible to update this job to a more recent version of Java?

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.

@toddbaert

toddbaert commented Sep 29, 2023

Copy link
Copy Markdown
Member

is it possible to update this job to a more recent version of Java?

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.

@liran2000

Copy link
Copy Markdown
Member Author

@markphelps opened PR for Flipt SDK.
There seems to be only the one mentioned code place to update, so changing the main branch and release seems simpler than a separate release for Java 8.
You can review, merge and release it if you agree for proceeding. Notice the release job itself has to compile with Java 8 too.
Thanks

Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
@markphelps

Copy link
Copy Markdown
Contributor

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

@liran2000

liran2000 commented Oct 11, 2023

Copy link
Copy Markdown
Member Author

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

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:

dev.openfeature.contrib.providers.flipt.FliptProviderTest
Error: dev.openfeature.contrib.providers.flipt.FliptProviderTest -- Time elapsed: 1.066 s <<< ERROR!
java.lang.NoSuchMethodError: java.util.Map.of(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/util/Map;

In the link provided, it is mentioned:

"Additionally, the source code cannot contain lambda expressions or any feature not available in Java 6."

I don't see how it can compile and run with Java 8 without removing the incompatible Map.of from source code.

@dsinghvi

Copy link
Copy Markdown

@liran2000 thanks for testing -- this was oversight on my part. Will rerelease a version of the generator today.

@markphelps markphelps left a comment

Copy link
Copy Markdown
Contributor

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 !!

Comment thread providers/flipt/pom.xml Outdated
liran2000 and others added 2 commits October 19, 2023 16:43
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>
@liran2000 liran2000 marked this pull request as ready for review October 19, 2023 13:53
@liran2000 liran2000 requested a review from a team as a code owner October 19, 2023 13:53
@beeme1mr beeme1mr requested a review from toddbaert October 23, 2023 17:28
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
@toddbaert

Copy link
Copy Markdown
Member

I'm ready to approve once the above unresolved comments are resolved.

@liran2000

Copy link
Copy Markdown
Member Author

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>
@toddbaert

Copy link
Copy Markdown
Member

I'm ready to approve once the above unresolved comments are resolved.

Comments are addressed in recents commits, can you check latest code?

Oh I'm sorry, I kept following sent to old permalinks to previous commits! Yes I see they are resolved. Approving!

@toddbaert toddbaert self-requested a review October 24, 2023 18:43
@toddbaert toddbaert merged commit dee6529 into open-feature:main Oct 24, 2023
@liran2000 liran2000 deleted the feature/flipt branch October 24, 2023 18:55
@markphelps

Copy link
Copy Markdown
Contributor

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

DBlanchard88 pushed a commit to DBlanchard88/java-sdk-contrib that referenced this pull request Apr 29, 2024
…n-feature#461)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
mahpatil pushed a commit to mahpatil/java-sdk-contrib that referenced this pull request Jun 3, 2026
Signed-off-by: Liran M <77168114+liran2000@users.noreply.github.com>
Co-authored-by: Mark Phelps <209477+markphelps@users.noreply.github.com>
Co-authored-by: Michael Beemer <beeme1mr@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.

5 participants