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.

Sign up for GitHub

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

Jump to bottom

adding retries to task circuit breaker #383

Closed
wants to merge 4 commits into from
Closed

adding retries to task circuit breaker #383

wants to merge 4 commits into from

Conversation

kdrakon
Copy link

@kdrakon kdrakon commented Jul 2, 2017

See #382 for reasoning behind this change

Sean Policarpio added 3 commits July 2, 2017 00:52
added new method protectWithRetry for TaskCircuitBreaker
0db1eba 
This allows a protected task to be automatically retried on the same
breaker. It also utilises the internal reset timeout of the breaker to
delay retried executions.
add documentation to protectWithRetry
577d697
added onErrorHandleWith partial function to protectWithRetry
28fd2b7
@alexandru
Copy link
Member

@kdrakon I'm still trying to digest this PR.

Can you please merge with master? It should fix the build failure.

@kdrakon
Copy link
Author

kdrakon commented Jul 19, 2017

hey @alexandru, sorry, I'm not sure why my branch keeps breaking with the openssl error :(

@alexandru
Copy link
Member

I think it's my setup to blame. Try and re-merge with master.

@alexandru
Copy link
Member

The OpenSSL problem is definitely my problem, but should be fixed on master.

@kdrakon
Copy link
Author

kdrakon commented Jul 21, 2017

@alexandru no worries. I already merged master though...still same failure

@alexandru
Copy link
Member

@kdrakon sorry for not replying, just adding a note that I'm on vacation and will give this some thought when I'll be back on Monday.

@kdrakon
Copy link
Author

kdrakon commented Jul 28, 2017

@alexandru no worries and no rush. Enjoy the vacation time 😊

@Avasil Avasil mentioned this pull request Oct 15, 2018
Merged
@alexandru
Copy link
Member

Hey @kdrakon, this took a really long time, being bad management on my part.

I think that the latest monix.catnap.CircuitBreaker has a facility required to do retries via a function being added to that interface, named awaitClose. I think it is simpler and more composable than protectWithRetry, but I did have you in mind when adding it 🙂

See doc sample: https://github.com/monix/monix/blob/master/monix-catnap/shared/src/main/scala/monix/catnap/CircuitBreaker.scala#L152

Again, sorry for the delay.

@alexandru alexandru closed this Nov 1, 2018
@kdrakon
Copy link
Author

kdrakon commented Nov 1, 2018

No worries @alexandru, and cheers for pointing me to the doc example 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kdrakon @alexandru