Skip to content
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

Make use of a request factory for filter/interceptor #732

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Make use of a request factory for filter/interceptor #732

wants to merge 2 commits into from

Conversation

m11r9000
Copy link

This PR involves creating a request factory that will be used to convert incoming HTTP requests from ReactorLoadBalancerExchangeFilterFunction/LoadBalancerInterceptor to org.springframework.cloud.client.loadbalancer.reactive.Request.

RequestFactory has one method that allows developers to plug in their own implementation for doing the conversion ClientRequest -> Request<?> or HttpRequest -> Request<?> or any other request object for that matter. It also contains default implementations that represents the default strategy of checking the loadBalancerHint.

@pivotal-issuemaster
Copy link

@andaziar Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@andaziar Thank you for signing the Contributor License Agreement!

@m11r9000 m11r9000 marked this pull request as draft April 14, 2020 13:46
@spencergibb
Copy link
Member

@andersenleo
Copy link

@spencergibb this PR was (is) related to the discussion in #672 -- it's not obvious to me how the referenced PR solved the issue of extracting a hint (or a concrete Request<T> from a http request (or I'm missing something)?

@spencergibb
Copy link
Member

The line I linked to has a create request method that can be implemented. Anyway, I think it's too early to consider something like this until #672 is done. I can reopen and put it on hold.

@andersenleo
Copy link

The line I linked to has a create request method that can be implemented

Thanks for the quick response. Our purpose was to inject the strategy in the WebClient/RestTemplate filter/interceptor. The createRequest() in the linked issue seems to be scoped to the LoadBalancer (at which point the http-request context is gone)?

Either way - as mentioned in #672 - we can find a way to work around this in our own code, but it would be much better if our work-around was somewhat aligned to your future design :)

@spencergibb
Copy link
Member

Sure. We can revisit after #672.

@spencergibb
Copy link
Member

The build fails BTW

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:compile (default-compile) on project spring-cloud-commons: Compilation failure

[ERROR] /home/appuser/project/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactiveLoadBalancer.java:[49,9] Unexpected @FunctionalInterface annotation

[ERROR]   org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer.Factory is not a functional interface

[ERROR]     multiple non-overriding abstract methods found in interface org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer.Factory

[ERROR] -> [Help 1]

@m11r9000
Copy link
Author

Hi and thank you for the quick replies @spencergibb . I'll fix the compile errors in this PR. Apologies for that. Sloppiness on my end.

And for clarifying regarding intent of this PR; it's a suggestion to showcase how one could extract the loadbalancer hint from the incoming request while also giving developers the power to do so as they see fit e.g. checking a header called loadBalancerHint or any other strategy that suits them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants