-
Notifications
You must be signed in to change notification settings - Fork 354
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
provides well documented lease example #840
Conversation
4b71165
to
2680d32
Compare
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
2680d32
to
c87365b
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.
Nice example. I've added a few suggestions below.
Do we still need the LeaseExample
? It's not connected to the lease notifications from the remote it seems, and retries blindly every second. In any case it doesn't seem to demonstrate anything extra so perhaps this can just replace it.
try { | ||
while (!Thread.currentThread().isInterrupted()) { | ||
String message = messagesQueue.poll(Long.MAX_VALUE, TimeUnit.DAYS); | ||
System.out.println("Process message {" + message + "}"); |
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.
Replace all System.out
with a Logger
.
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.
fixed
...ples/src/main/java/io/rsocket/examples/transport/tcp/lease/RateLimitingWithLeaseExample.java
Outdated
Show resolved
Hide resolved
...ples/src/main/java/io/rsocket/examples/transport/tcp/lease/RateLimitingWithLeaseExample.java
Outdated
Show resolved
Hide resolved
String.format("%s stats are %s", tag, leaseStats.isPresent() ? "present" : "absent")); | ||
Duration ttlDuration = Duration.ofSeconds(5); | ||
// The interval function is used only for the demo purpose and should not be | ||
// considered as the way to issue leases. |
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.
It would be helpful to describe what would be used instead of interval
?
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.
…cp/lease/RateLimitingWithLeaseExample.java Co-authored-by: Rossen Stoyanchev <rstoyanchev@users.noreply.github.com>
Co-authored-by: Rossen Stoyanchev <rstoyanchev@users.noreply.github.com> Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
Co-authored-by: Rossen Stoyanchev <rstoyanchev@users.noreply.github.com> Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
Provides a good example which shows how Lease can be used for the straight-forward rate-limiting of FireAndForget calls
Signed-off-by: Oleh Dokuka shadowgun@i.ua