Allow adding ClientInterceptors to specific services and methods
#2113
Conversation
This file contains 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
gjcairo
commented
Nov 13, 2024
Sources/GRPCCore/GRPCClient.swift
Outdated
| /// | ||
| /// The order in which interceptors are added reflects the order in which they are called. The | ||
| /// first interceptor added will be the first interceptor to intercept each request. The last | ||
| /// interceptor added will be the final interceptor to intercept each request before calling | ||
| /// the appropriate handler. | ||
| private let interceptors: [any ClientInterceptor] | ||
| private let interceptorsPerMethod: Mutex<[MethodDescriptor: [any ClientInterceptor]]> |
There was a problem hiding this comment.
I thought this caching could be useful to speed things up, similar to what we do in the RPCRouter on the server. However I wonder if we care more about the client not taking up too much space (which could be the case if it calls many methods and this dictionary grows a lot).
glbrntt
reviewed
Nov 14, 2024
glbrntt
requested changes
Nov 15, 2024
Tests/GRPCCoreTests/Call/Client/ClientInterceptorPipelineOperationTests.swift
Outdated
Show resolved
Hide resolved
Tests/GRPCCoreTests/Call/Client/ClientInterceptorPipelineOperationTests.swift
Outdated
Show resolved
Hide resolved
glbrntt
approved these changes
Nov 15, 2024
gjcairo
force-pushed
the
service-specific-client-interceptors
branch
from
0329e42
to
2bf4d02
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
We want to allow users to customise the RPCs a registered interceptor should apply to on the client:
Modifications
This PR adds a new
ClientInterceptorPipelineOperationtype that allows users to specify what the target of the interceptor should be.Existing APIs accepting
[any ClientInterceptor]have been kept, but new initialisers taking[ClientInterceptorPipelineOperation]instead have been added.Result
Users can have more control over to which requests interceptors are applied.