fix(spring): limit retry settings configuration by method type #1212
Conversation
emmileaf
added
the
spring
emmileaf
changed the title
| @@ -659,10 +659,13 @@ private static MethodDefinition createSettingsBeanMethod( | |||
| List<Statement> updateRetrySettingsStatementBody = new ArrayList<>(); | |||
|
|
|||
| for (Method method : service.methods()) { | |||
| List<Statement> updateMethodWithServiceRetryStatments = | |||
| if (!method.stream().equals(Method.Stream.NONE) || method.hasLro()) { | |||
There was a problem hiding this comment.
I think it would make sense to filter the methods in the foreach declaration (e.g. for (Method method : Utils.getFilteredMethods(service.methods)) to centralize/dereplicate the filtering logic (could also be done by adding a convenience method in Service but may be overkill)
There was a problem hiding this comment.
Makes sense, given we expect this to be changed in the future. Updated and moved this logic to utils.
|
Kudos, SonarCloud Quality Gate passed! |
| public static List<Method> getMethodsForRetryConfiguration(Service service) { | ||
| // Returns list of methods with retry configuration support | ||
| // This currently excludes streaming and LRO methods | ||
| return service.methods().stream() |
There was a problem hiding this comment.
Just a note that we need to revisit this logic before GA, as we discussed, BIDI and Client streaming do not support retry, but Server streaming and LRO should support it.
There was a problem hiding this comment.
Noted - thank you!
This PR is a patch for the
RetrySettingsconfiguration issue discovered in GoogleCloudPlatform/spring-cloud-gcp#1407 (comment). It excludes streaming and LRO methods from retry configuration through spring properties for now.