Recommend fault tolerance instead of max-retries

[geoand]

No description provided.

[edeandrea]

I actually prefer the property configuration of the retries. It allows for customizing the retry logic per deployment (dev mode vs unit test vs integration test vs prod...)

[edeandrea]

What about with the timeouts as well? There are default timeouts set on the underlying Langchain4J calls that would need to be translated to use SmallRye Fault tolerance annotations?

Now you'll also have competing priorities. I'd prefer using the framework-provided config options rather than fault tolerance annotations.

If you don't want users to use the config options, then don't expose them.

But honestly, I like the ability to configure retries/timeouts via config rather than annotations.

[geoand]

I actually prefer the property configuration of the retries. It allows for customizing the retry logic per deployment (dev mode vs unit test vs integration test vs prod...)

You can provide a configuration property in the Smallrye config annotations, right?

[edeandrea]

You can provide a configuration property in the Smallrye config annotations, right?

You can?

[geoand]

I think so, but I might be misremembering

[edeandrea]

According to https://quarkus.io/guides/smallrye-fault-tolerance it looks like you can for retries, but its kinda ugly. The configuration key has to contain both the fully-qualified class name & method name...

Even the SmallRye docs don't mention that you can do it for timeouts...

package org.acme;

import org.eclipse.microprofile.faulttolerance.Retry;
...

public class CoffeeResource {
    ...
    @GET
    @Retry(maxRetries = 4)
    @Timeout(value = 4, unit = ChronoUnit.SECONDS)
    public List<Coffee> coffees() {
        
    }
    ...
}

and then

org.acme.CoffeeResource/coffees/Retry/maxRetries=6

That being said, I think enforcing timeouts one way but retries another is clunky to be honest.

[geoand]

Hm, @Ladicek would know for sure if we have better config property resolution support in Quarkus

[Ladicek]

There's a feature request for better config properties for Fault Tolerance, but nothing specific has been done about that yet. The main issue is that we don't really have an identifier we could use (akin to e.g. RestClient's configKey).

And for the record, even though we only show retries in the documentation of Fault Tolerance configuration, the exact same thing applies to all Fault Tolerance annotations, even those that come from SmallRye.

[geoand]

Thanks for the information

[edeandrea]

And for the record, even though we only show retries in the documentation of Fault Tolerance configuration, the exact same thing applies to all Fault Tolerance annotations, even those that come from SmallRye.

Say I have

@Timeout(value = 4, unit = ChronoUnit.SECONDS)

and via config I wanted to change value=6 and unit = ChronoUnit.MILLIS - how would I do that via properties?

org.acme.CoffeeResource/coffees/Timeout/value=6
org.acme.CoffeeResource/coffees/Timeout/unit=?????

[Ladicek]

ChronoUnit is an enum, so whatever works for enums elsewhere, works here as well. MILLIS should work and maybe even millis (not sure about that).

[edeandrea]

So

org.acme.CoffeeResource/coffees/Timeout/value=6
org.acme.CoffeeResource/coffees/Timeout/unit=MILLIS

should work? Or does ChronoUnit. need to be there?

org.acme.CoffeeResource/coffees/Timeout/value=6
org.acme.CoffeeResource/coffees/Timeout/unit=ChronoUnit.MILLIS

[Ladicek]

So

org.acme.CoffeeResource/coffees/Timeout/value=6
org.acme.CoffeeResource/coffees/Timeout/unit=MILLIS

should work?

Absolutely.

Or does ChronoUnit. need to be there?

org.acme.CoffeeResource/coffees/Timeout/value=6
org.acme.CoffeeResource/coffees/Timeout/unit=ChronoUnit.MILLIS

No.

[jmartisk]

@geoand, can we close this now that we have #507 in?

[geoand]

Good point!