Skip to content

fix: change watch API#104

Closed
belltoy wants to merge 1 commit into
etcdv3:masterfrom
belltoy:fix/watch
Closed

fix: change watch API#104
belltoy wants to merge 1 commit into
etcdv3:masterfrom
belltoy:fix/watch

Conversation

@belltoy

@belltoy belltoy commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

The current WatchClient::watch function await the first response to the create request and keep the watch_id in the Watcher. This is not necessary, either a good idea for a etcd watch API.

The etcd Watch API behaves differently than other unary RPC. It doesn't send grpc-status in the trailers if something goes wrong. Instead, we must solve the result via the WatchResponse and its corresponding request.

Changes

The watch function returns a tuple (WatchResponse, Watcher, WatchStream), the first element in the returned tuple is the response to the first create watch request.

The Watcher struct doesn't hold the first watch_id inside. The Watcher::cancel function will always need a watch_id as parameter.

The returned WatchResponse to the first WatchCreateRequest includes more useful information to upstream applications. For example, if the request with a particular start_revision which may have been compacted in the server, then the client should never retry with the same start_revision.

BTW, in this PR, I still await the first response and try to solve the result of the first create request. But I prefer leaving it to the higher level watcher: belltoy#1

@belltoy belltoy force-pushed the fix/watch branch 2 times, most recently from 5ba8852 to 9017eac Compare June 11, 2025 06:20
@belltoy

belltoy commented Jun 25, 2025

Copy link
Copy Markdown
Contributor Author
 error: the `Err`-variant returned from this function is very large
    --> src/client.rs:156:71
     |
 156 |     fn build_endpoint(url: &str, options: &Option<ConnectOptions>) -> Result<Endpoint> {
     |                                                                       ^^^^^^^^^^^^^^^^
     |
    ::: src/error.rs:24:5
     |
 24  |     GRpcStatus(tonic::Status),
     |     ------------------------- the largest variant contains at least 176 bytes
     |
     = help: try reducing the size of `error::Error`, for example by boxing large elements or replacing it with `Box<error::Error>`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
     = note: `-D clippy::result-large-err` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::result_large_err)]`

Failure in the clippy check, but didn't change the Error type. Found that warning was shows from rust 1.87.0. grpc/grpc-rust#2253

@belltoy belltoy force-pushed the fix/watch branch 2 times, most recently from 051365b to 34de556 Compare June 28, 2025 00:32
@belltoy belltoy mentioned this pull request Jul 9, 2025
@belltoy belltoy added v0.16 enhancement New feature or request and removed v0.16 labels Jul 9, 2025
@belltoy belltoy added this to the v0.17 milestone Jul 15, 2025
@belltoy belltoy removed the v0.17 label Jul 15, 2025
@belltoy belltoy modified the milestones: v0.17, v0.18 Sep 20, 2025
The `watch` function returns a tuple `(WatchResponse, Watcher, WatchStream)`, the first element
in the returned tuple is the response to the first create watch request.

More importantly, the etcd `Watch` API behaves differently than other unary RPC. It doesn't
send grpc-status in the trailers if something goes wrong. Instead, we must solve the result
via the `WatchResponse` and its corresponding request.
@belltoy

belltoy commented Jan 7, 2026

Copy link
Copy Markdown
Contributor Author

use #108

@belltoy belltoy closed this Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant