clientv3: document context to "Snapshot" API #9024
Conversation
gyuho
changed the title
clientv3/maintenance.go
Outdated
| @@ -55,6 +55,11 @@ type Maintenance interface { | |||
| HashKV(ctx context.Context, endpoint string, rev int64) (*HashKVResponse, error) | |||
|
|
|||
| // Snapshot provides a reader for a point-in-time snapshot of etcd. | |||
| // If the context "ctx" is canceled or timed out, reading from returned | |||
| // "io.ReadCloser" would error, and its error type is "grpc/*status.statusError". | |||
There was a problem hiding this comment.
should we add some sort test to ensure the error message and type is what we believed? I suspect if grpc decides to change underlying error and type, the error message and type could be different.
On the other hand, maybe just let the user knows that ctx cancel/timeout will error out io.ReadCloser is sufficient?
// If the context "ctx" is canceled or timed out, reading from returned
// "io.ReadCloser" would error out.
There was a problem hiding this comment.
Yeah the error is always grpc/*status.statusError type, and we don't have tests around it. Now I think it's better to translate into context.Canceled or context.DeadlineExceeded as other APIs do. What do you think? /cc @xiang90
There was a problem hiding this comment.
it seems like this added doc string is too detailed. we don't really have this for other apis. should we just keep the doc string a bit more general?
|
lgtm. |
Codecov Report
@@ Coverage Diff @@
## master #9024 +/- ##
=========================================
Coverage ? 76.04%
=========================================
Files ? 359
Lines ? 29838
Branches ? 0
=========================================
Hits ? 22689
Misses ? 5573
Partials ? 1576
Continue to review full report at Codecov.
|
Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
|
@gyuho can we get this merged? |
Address #8993.