-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add retry feature to async feign #1757
Add retry feature to async feign #1757
Conversation
9e1ac8c
to
46c098b
Compare
c6966b0
to
0633e1e
Compare
0633e1e
to
1379437
Compare
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.
@kdavisk6 do you want to take a look too?
I think this will break binary compatibility, so, major release?
@velo What classes are you concerned about breaking binary compatibility?(I know a little about Java binary compatibility) If you're concerned about breaking binary compatibility for the Async feature, isn't it ok because it's experimental? |
Yeah, it's ok. But, there will be complaints:/ |
@velo What changes break AsyncFeign's Binary Compatability? If the problem is the change from abstract to final, how about removing the final keyword? After that, how about adding the final keyword as a separate PR? |
Are you already planning to release the coroutine Feign to version 12? 45fe10a However, in the process of raising the stability level from the experimental version to the stable, there may be several breaking changes in the public API, and I am a little worried about whether it would be okay to raise the major version each that time. If Feign's major version has been changed several times with only changes to AsyncFeign, users might feel weird. I agree there will be complaints, but I think experimental imply that it could take a breaking change. |
The MethodInfo.configKey field is not more used since PR OpenFeign#1757 removing AsyncInvocation
The MethodInfo.configKey field is not more used since PR #1757 removing AsyncInvocation Co-authored-by: Marvin Froeder <[email protected]>
False negative test case is added in PR OpenFeign#1757
False negative test case is added in PR #1757 Co-authored-by: Marvin Froeder <[email protected]>
* Define MethodHandler.Factory interface * Extract AsynchronousMethodHandler from SynchronousMethodHandler * Genericize AsynchronousMethodHandler for receive requestContext * Pass requestContext to AsynchronousMethodHandler * Add retry feature to AsyncFeign * Remove ReflectiveAsyncFeign Co-authored-by: Marvin Froeder <[email protected]>
The MethodInfo.configKey field is not more used since PR #1757 removing AsyncInvocation Co-authored-by: Marvin Froeder <[email protected]>
False negative test case is added in PR #1757 Co-authored-by: Marvin Froeder <[email protected]>
* Define MethodHandler.Factory interface * Extract AsynchronousMethodHandler from SynchronousMethodHandler * Genericize AsynchronousMethodHandler for receive requestContext * Pass requestContext to AsynchronousMethodHandler * Add retry feature to AsyncFeign * Remove ReflectiveAsyncFeign Co-authored-by: Marvin Froeder <[email protected]>
The MethodInfo.configKey field is not more used since PR #1757 removing AsyncInvocation Co-authored-by: Marvin Froeder <[email protected]>
False negative test case is added in PR #1757 Co-authored-by: Marvin Froeder <[email protected]>
Resolves #1758
Contains #1756 PR commits