Conversation
miparnisari
force-pushed
the
add-watch-code-snippet
branch
3 times, most recently
from
9b3e0b0 to
f915ccd
Compare
tstirrat15
left a comment
tstirrat15
left a comment
There was a problem hiding this comment.
Can we either put it in the examples directory or additionally put it in the examples dir? https://github.com/authzed/authzed-java/tree/main/examples/v1
|
@tstirrat15 why "additionally"? What value does having it in the "examples" directory bring? Personal opinion, but it seems quite hidden |
miparnisari
force-pushed
the
add-watch-code-snippet
branch
from
f915ccd to
393cf64
Compare
README.md
Outdated
| } | ||
| ``` | ||
|
|
||
| ### Calling `Watch` |
There was a problem hiding this comment.
Can we perhaps move this to the examples directory so it can be compiled and updated as needed? Then we can link the example from the readme. Same for check permissions.
README.md
Outdated
| builder.setOptionalStartCursor(lastZedToken); | ||
| } | ||
|
|
||
| WatchRequest request = builder.build(); |
There was a problem hiding this comment.
I'd suggest enabling heartbeats. This solves the confusion around idle timeouts and long-lived gRPC streaming APIs.
There was a problem hiding this comment.
I need to update the API first.
README.md
Outdated
|
|
||
| } catch (Exception e) { | ||
| if (e instanceof StatusRuntimeException sre && sre.getStatus().getCode().equals(Status.UNAVAILABLE.getCode()) && | ||
| (sre.getMessage().contains("stream timeout") || sre.getMessage().contains("RST_STREAM closed stream"))) { |
There was a problem hiding this comment.
We shouldn't be checking the message to trigger retries - these could be subject to change. I assume those have INTERNAL grpc code, so those are safe to be retried.
In addition to this manual handling, we could add retry policies, which is built into the Java gRPC SDK. Please note that retry policies are only supported for server-streaming APIs (like the Watch API), but are not supported for client-streaming and bidirectional APIs. I'm also not fully sure server-streaming is retried once data has been streamed back to the client.
|
@miparnisari i realize that it isn't often a place you would look, but when it's in a |
miparnisari
force-pushed
the
add-watch-code-snippet
branch
2 times, most recently
from
a13c387 to
fa39172
Compare
miparnisari
force-pushed
the
add-watch-code-snippet
branch
2 times, most recently
from
7add8f9 to
6095dc4
Compare
miparnisari
force-pushed
the
add-watch-code-snippet
branch
from
6095dc4 to
f239482
Compare
|
|
||
| } catch (Exception e) { | ||
| if (e instanceof StatusRuntimeException sre && (sre.getStatus().getCode().equals(Status.UNAVAILABLE.getCode()) || | ||
| (sre.getStatus().getCode().equals(Status.INTERNAL.getCode())) && sre.getMessage().contains("stream timeout"))) { |
There was a problem hiding this comment.
IMO we should just do this on the Internal code alone
There was a problem hiding this comment.
shouldn't we also retry on Unavailable?
ecordell
left a comment
ecordell
left a comment
There was a problem hiding this comment.
LGTM, with the caveat I haven't run this locally myself. But we can follow up if we find issues.
4 participants
Description
Streaming APIs such as Watch can get disconnected. This PR provides a code snippet that can be used to automatically re-connect.
Testing
Locally, with https://github.com/miparnisari/authzed-java-client-test