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

Fault Tolerance support in builder created client #347

Open
Verdent opened this issue May 31, 2022 · 4 comments
Open

Fault Tolerance support in builder created client #347

Verdent opened this issue May 31, 2022 · 4 comments
Labels
Milestone

Comments

@Verdent
Copy link

Verdent commented May 31, 2022

I would like to ask you, whether it is expected for clients created via builder (not CDI), to support FT CDI interceptor bindings. There is no TCK test which would verify such a behavior and therefore I am not sure whether it is actually required.

@andymc12
Copy link
Contributor

andymc12 commented Jun 1, 2022

The short answer is yes - FT CDI interceptor bindings should be honored - even for programmatically-created client instances. This is the text I would refer to:

If CDI is available, the MP Rest Client implementation must ensure that CDI business method interceptors are invoked when the appropriate interceptor binding is applied to the client interface or method.

Also:

MicroProfile Fault Tolerance
MP Rest Client implementations must ensure that MP Fault Tolerance annotations on client interfaces are honored. In general, these annotations are treated as CDI interceptor bindings.

MP Rest Client should ensure that the behavior of most Fault Tolerance annotations should follow the behavior outlined in the MP Fault Tolerance specification. This includes the @Asynchronous, @Bulkhead, @CircuitBreaker, @Fallback and @Retry annotations.

The @Timeout annotation presents a problem since some parts of the MP Rest Client request are non-blocking and non-interruptible. Implementations should override the default connect and read timeouts and use the timeout value specified in the @Timeout annotation instead. This will ensure that the actual time spent in blocking/non-interruptible operations should be less than or equal to the time specified in the annotation, allowing the MP Fault Tolerance implementation to interrupt the request and the throw the appropriate TimeoutException.

Neither texts says that FT interceptors are limited only to CDI-injected interfaces.

I think it would be good to add a test to CDIInterceptorTest to use a programmatic client in addition to the already-tested CDI-injected client.

@tomas-langer
Copy link

I can understand the CDI interceptors work for injected cases.
I do not clearly understand how the interaction with Buider works.
Considering that the builder can override most of the configuration (including endpoint), build cannot use an instance managed by CDI (as it would get an instance configured with different configuration). I can imagine this working in case the injection would be always @Dependent and that we could somehow get the configuration from builder to the CDI factory method, but not when a bean can be @ApplicationScoped.
As a result, instances created by builder cannot have CDI interceptors, as they cannot be managed by CDI.

I am wondering how you expect to get CDI interceptors for instances not created and managed by CDI?

@andymc12
Copy link
Contributor

andymc12 commented Jun 1, 2022

Open Liberty supports CDI interceptors from programmatic clients. Perhaps you could look at their code (and the CXF code it is built on) to see how it works? That said, CDI has a variety of ways to get interceptors for specified bindings - or you could go all the way of making the programmatically-created instance be managed by CDI.

I'm no longer with IBM, and so I haven't been too much involved in MP Rest Client lately, so if the community would prefer that CDI interceptors be limited to CDI-created/managed client instances, it is fine with me. :-)

@Emily-Jiang
Copy link
Member

This needs further discussion and investigation.

@Emily-Jiang Emily-Jiang added this to the Future milestone Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants