-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix default handler overide , when user passed in interceptor should … #1609
Conversation
…overide any default implementations
@baywet @Ndiritu can you please review this , i have added a new default interceptor to be in sync with .Net code , Net pull Request |
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.
Thank you for your time on this @raghucha
I think a cleaner override workflow would be, a developer gets the default Interceptor list... and can re-arrange it as they please and when they pass it as a parameter, we add the interceptors as is to the OkHttpClient builder:
var interceptors = createDefaultInterceptors();
// dev manipulates interceptors as needed, pre-pend/append custom Interceptors, remove, re-arrange etc...
KiotaClientFactory.create(interceptors); // doesn't manipulate interceptors but adds to the builder directly
Or they pass custom configuration options for the existing default Interceptors by adding
createDefaultInterceptors(List<RequestOption> requestOptions)
overload, which we can add in this PR. We can then match the different options and pass them to the corresponding interceptors.
Happy to help where needed.
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
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.
Thanks for the contribution!
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java
Outdated
Show resolved
Hide resolved
…KiotaClientFactory.java Co-authored-by: Vincent Biret <vincentbiret@hotmail.com>
…KiotaClientFactory.java Co-authored-by: Vincent Biret <vincentbiret@hotmail.com>
…KiotaClientFactory.java Co-authored-by: Vincent Biret <vincentbiret@hotmail.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
fix: deprecates extraneous method Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
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.
Thank you for making the changes!
I made additional changes to ensure we provide a consistent API surface.
@raghucha can you please accept the license agreement? (see the bot message at second comment) |
@microsoft-github-policy-service agree |
This Pull request allows us to overide the default implementations of the interceptor behaviour . This is linked to this bug