-
Notifications
You must be signed in to change notification settings - Fork 75
feat: streaming example #322
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
Conversation
} | ||
|
||
dependencies { | ||
implementation("io.getunleash:unleash-client-java:11.1.0") |
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.
version with streaming required
|
||
dependencies { | ||
implementation("io.getunleash:unleash-client-java:11.1.0") | ||
implementation("com.launchdarkly:okhttp-eventsource:4.1.1") |
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.
optional dependency for streaming
Pull Request Test Coverage Report for Build 17093964056Details
💛 - Coveralls |
I manually tested it against a server without streaming support, and it doesn't fall back to polling. I don't know if it's the intended behavior but it does use the backup to load the initial state but it does not fetch from upstream. I double-checked, and it's not polling. I believe it should poll if streaming fails. |
Against a server that supports streaming:
The main difference with a server that doesn't support streaming is that the client will output:
Which probably means the error is not handled (because disabling streaming works). |
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.
This looks good! The happy path works well. We know about this limitation #322 (comment) but it will be addressed later. Meanwhile, the features is clearly marked as experimental which means, it should not be used in production yet. Good step forward!
System.out.println("Streaming client started. Waiting for events... (Press Ctrl+C to exit)"); | ||
|
||
try { | ||
Thread.currentThread().join(); | ||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); | ||
System.out.println("Streaming example shutting down"); | ||
} | ||
} |
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 ok, I'd like to also get some feature evaluation result, but I tested that manually and it works well.
I did something similar to this:
unleash-java-sdk/examples/cli-example/src/main/java/io/getunleash/example/AdvancedConstraints.java
Lines 29 to 32 in 41c6028
while (true) { | |
unleash.isEnabled("advanced.constraints", context); // expect this to be true | |
unleash.isEnabled("advanced.constraints", smallerSemver); // expect this to be false | |
} |
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.
There's boolean isEnabled = unleash.isEnabled("streaming_flag");
which is a flag you need to create and you will see it changing instantly on every streaming change. This is a similar pattern I did in Node SDK where we react to instant changes instead of looping which was fine for the polling mindset but for streaming event listening mindset seems to be more appropriate for me.
About the changes
Now that we released a new version of Java SDK with streaming support we can also add an example.
Run it with UNLEASH_API_TOKEN=sandbox_token UNLEASH_API_URL=sandbox_url ./gradlew run
The example listens to streaming changes and prints flag status and received events.
Most of the PR is boilerplate for gradle copied from other examples. Actual change is 60 LOC
Important files
Discussion points