thrift: support onLocalReply to inform filters of local replies#21387
thrift: support onLocalReply to inform filters of local replies#21387zuercher merged 6 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com>
Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com>
|
|
||
| // Continue sending onLocalReply to all filters, but reset the stream once all filters have been | ||
| // informed rather than sending the local reply. | ||
| ContinueAndResetStream, |
There was a problem hiding this comment.
Is the description correct? Seems like parent_.sendLocalReply is always called and this causes the stream to be reset after the reply.
There was a problem hiding this comment.
you're right. Let me see which makes sense, changing description or implementation.
Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com>
|
After more deliberation, It would make more sense to let filter stop the response instead of ending stream. To reduce risk, I'd like to remove the code for end stream handling for now, @zuercher would you mind to take another look? Thanks! |
|
/retest |
|
Retrying Azure Pipelines: |
zuercher
left a comment
zuercher
left a comment
There was a problem hiding this comment.
One small thing on the release notes, but otherwise looks good to me.
Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com>
|
/retest |
|
Retrying Azure Pipelines: |
…yproxy#21387) Notifies filters of local replies. Risk Level: low, defaults to no-op Testing: unit tests added Docs Changes: doxygen comments Release Notes: added Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com> Signed-off-by: Amila Senadheera <amila.15@cse.mrt.ac.lk>
3 participants
Signed-off-by: kuochunghsu kuochunghsu@pinterest.com
Commit Message:
Similar setup with http onLocalReply in #15172
Additional Description:
Risk Level: low
Testing: unit test
Docs Changes: current.yaml
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]