-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Update ServerInHandle comments #1437
Conversation
tap/tap.go
Outdated
// ServerInHandle defines the function which runs before a new stream is created | ||
// on the server side. If it returns a non-nil error, the stream will not be | ||
// created and a RST_STREAM will be sent back to the client with REFUSED_STREAM. | ||
// The client will receive a RPC error "code = Unavailable, desc = stream |
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.
Nit: an RPC error
tap/tap.go
Outdated
// terminated by RST_STREAM with error code: REFUSED_STREAM". | ||
// | ||
// It's intended to be used in situations where you don't want to waste the | ||
// resource to accept the new stream (e.g. rate-limiting). And the content of |
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.
Nit: resources
tap/tap.go
Outdated
// Note that it is executed in the per-connection I/O goroutine(s) instead of | ||
// per-RPC goroutine. Therefore, users should NOT have any | ||
// blocking/time-consuming work in this handle. Otherwise all the RPCs would | ||
// slow 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.
Should we add a note about how that means it also won't be called multiple times at once, and so doesn't need to be thread-safe?
f4289cf
to
8b411fd
Compare
8b411fd
to
1747f34
Compare
Thanks for the review. All fixed. PTAL. |
fixes #1436