[promises] Add an API to access new style filters - #35200
Closed
ctiller wants to merge 60 commits into
Closed
Conversation
The code in promise_based_filter allows http_client_filter to use the v3 APIs for filters. The implementation of the mapping from v3 -> v2.1 is as yet incomplete, but sufficient for this sample. The long term aim will be to convert all filters to this API, and then transition the runtime from the current promise based stack to a new stack that uses these interception points natively. PiperOrigin-RevId: 586188548
Automated fix for refs/heads/cg-http-cli
Automated fix for refs/heads/cg-http-svr
Automated fix for refs/heads/cg-channel-filter-api
grpc-checks
Bot
added
bloat/medium
per-call-memory/increase
and removed
per-call-memory/neutral
labels
Automated fix for refs/heads/cg-channel-filter-api
grpc-checks
Bot
added
per-call-memory/neutral
and removed
per-call-memory/increase
labels
…hannel-filter-api
grpc-checks
Bot
added
per-call-memory/increase
and removed
per-call-memory/neutral
labels
grpc-checks
Bot
added
per-call-memory/neutral
and removed
per-call-memory/increase
labels
markdroth
approved these changes
Dec 4, 2023
markdroth
left a comment
markdroth
left a comment
Member
There was a problem hiding this comment.
Looks good modulo comments!
| virtual Pipe<MessageHandle>& client_to_server_messages() = 0; | ||
| virtual Pipe<MessageHandle>& server_to_client_messages() = 0; | ||
| virtual Pipe<ServerMetadataHandle>& server_trailing_metadata() = 0; | ||
| GRPC_MUST_USE_RESULT virtual absl::nullopt_t Cancel( |
Member
There was a problem hiding this comment.
As discussed, please add a comment explaining why GRPC_MUST_USE_RESULT is being used with absl::nullopt_t, and maybe a TODO to find a more elegant way to accomplish the same thing.
|
|
||
| // TODO(ctiller): eventually drop this when we don't need to reference into | ||
| // legacy promise calls anymore | ||
| class CallSpineInterface { |
Member
There was a problem hiding this comment.
Please add a comment explaining the purpose of this class and its relationship to CallInitiator, CallHandler, top-level calls, client_channel calls, and server calls. Also its importance for cancellation.
| template <typename Derived> | ||
| class ImplementChannelFilter : public ChannelFilter { | ||
| public: | ||
| void InitCall(CallSpineInterface* call_spine) { |
Member
There was a problem hiding this comment.
Please add a comment explaining that this is doing v3->v2 conversion (ability to run a v3 filter in a v2 stack), and that it transitively works for v3->v1 because we have v2->v1 already implemented.
markdroth
approved these changes
Dec 4, 2023
grpc-checks
Bot
added
per-call-memory/increase
and removed
per-call-memory/neutral
labels
yashykt
approved these changes
Dec 5, 2023
drfloob
approved these changes
Dec 5, 2023
grpc-checks
Bot
added
per-call-memory/neutral
and removed
per-call-memory/increase
labels
copybara-service Bot
pushed a commit
that referenced
this pull request
Dec 5, 2023
Will be used during this transition time to run 5-pipe style filters somewhat more natively. Once everything is getting closer to 5-pipes, we'll drop this method and have the channel stack understand how to create an interception-map that can be reused per-call, instead of creating the interception-map every time a call is created. Closes #35200 COPYBARA_INTEGRATE_REVIEW=#35200 from ctiller:cg-channel-filter-api 2fc11dd PiperOrigin-RevId: 587940947
paulosjca
pushed a commit
to paulosjca/grpc
that referenced
this pull request
Dec 8, 2023
Will be used during this transition time to run 5-pipe style filters somewhat more natively. Once everything is getting closer to 5-pipes, we'll drop this method and have the channel stack understand how to create an interception-map that can be reused per-call, instead of creating the interception-map every time a call is created. Closes grpc#35200 COPYBARA_INTEGRATE_REVIEW=grpc#35200 from ctiller:cg-channel-filter-api 2fc11dd PiperOrigin-RevId: 587940947
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Will be used during this transition time to run 5-pipe style filters somewhat more natively. Once everything is getting closer to 5-pipes, we'll drop this method and have the channel stack understand how to create an interception-map that can be reused per-call, instead of creating the interception-map every time a call is created.