-
Notifications
You must be signed in to change notification settings - Fork 646
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
Configurable request retry #2453
Configurable request retry #2453
Conversation
The main issue we can't use generic reactor-core retry concept is due to the inability to detect if HTTP-level data has been sent across the wire. reactor#2425
as opposed to some homegrown class. reactor#2425
@crankydillo Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
@crankydillo I'm gonna check this next week |
Thanks Violeta. Honestly, I need to work on it. It was not as easy as initially thought:) I can pass all the tests with my current code (not pushed yet), but I'm not happy about it, and I think there may be some corner cases I'm missing. I'll do my best to work on it a bit over the next week. |
I pushed it, but I'm not happy about it. It's going to be a busy week or two, so I'm not sure when I will come back to it. Bottom line is I couldn't figure out how to avoid some huge redesign to the observers and still do this in a clean way. The crux of the problem revolves around getting that done-retrying signal to the observers at the right time. The only other idea I have is to have an internal, failing always sink-based flux that fails always embedded within |
On a side note, I thought I'd already signed the Pivotal agreement. I'll get it signed again I guess. |
@violetagg I'm just going to close this. I understand the desire to leverage reactor's retry, but I don't see how that can quickly be done. Given that my team is not demanding this feature, I doubt I'll be able to resolve this. |
Allow more configuration when it comes to retrying requests.
This is currently failing at least 1 test for 2 reasons that need to be addressed. First,
Retry#max
exhaustion generates the retryexhausted wrapper. I think that's a good thing, but I understand why you'd go a different route from multiple perspectives (IO checked retry, breaking behavior). Still thinking on that, but feel free to tell me what to do.Also, in the error case, this fires the
doOnError
hook maxRetry + 1. Still looking into this.Need to stop for the day, but figured I'd get a draft out there. I'll clean up all commits/code/etc. before moving out of draft.
#2425