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.

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

Tests and recoveringWatchdog #4

Open
jcristovao opened this issue May 5, 2014 · 2 comments
Open

Tests and recoveringWatchdog #4

jcristovao opened this issue May 5, 2014 · 2 comments

Comments

@jcristovao
Copy link
Contributor

Hi,

I'm not raising this as a pull request because I'm unsure you would be ok with the changes I made:

  1. Added quicktest tests
  2. Added recoveringWatchdog function that makes a distintion between an aplication that fails very quickly each time (and thus the retry count applies), or an application that only fails ocasionally. In this last case, I want to retry it but reset the retry counter.

Number 2), unfortunately adds a lot of dependencies (you might not be confortable with).

https://github.com/jcristovao/retry/commits/master

If there is interest, I might split this into (mergeable) tests applicable to the retry lib (as it is), and the recoveringWatchdog I would split into a different lib.

Please let me know what do you prefer,
Cheers

@ozataman
Copy link
Member

ozataman commented May 5, 2014

Hello Joao,

A few comments:

  1. I would be very interested in merging your testsuite - thank you for
    that.

  2. I would like to stay away from monad-control family of handlers if
    possible, as they are a bit heavy handed and not every application uses
    them. "exceptions" OTOH, provide a narrower surface area.

  3. Your reset-count-after use case is an interesting one and I do see the
    point. I would likely implement it differently, however. I am not a fan of
    imperative MVar/IORef patterns unless absolutely necessary and the setting
    for this needs properly interop with the rest of RetrySettings. In fact, we
    do not even need a different function for this; it appears to me like a
    trivial addition to the existing retry combinators. I'll look into it when
    I get the chance.

As you've suggested; I would appreciate a cleaned up patch just for the
testing part and I'll take a look at the reset-after use case.

Thanks,
Oz

Ozgun Ataman

CEO

Soostone Inc.

+1 917 725 0849 (Office)
+1 734 262 0676 (Mobile)
+1 212 320 0251 (Fax)
[email protected]

On Mon, May 5, 2014 at 9:57 AM, João Cristóvão [email protected]:

Hi,

I'm not raising this as a pull request because I'm unsure you would be ok
with the changes I made:

  1. Added quicktest tests
  2. Added recoveringWatchdog function that makes a distintion between an
    aplication that fails very quickly each time (and thus the retry count
    applies), or an application that only fails ocasionally. In this last case,
    I want to retry it but reset the retry counter.

Number 2), unfortunately adds a lot of dependencies (you might not be
confortable with).

https://github.com/jcristovao/retry/commits/master

If there is interest, I might split this into (mergeable) tests applicable
to the retry lib (as it is), and the recoveringWatchdog I would split into
a different lib.

Please let me know what do you prefer,
Cheers


Reply to this email directly or view it on GitHubhttps://github.com//issues/4
.

@jcristovao
Copy link
Contributor Author

Hello Ozgun,

  1. I've submitted a pull request with the relevant tests, I hope its ok with you.

  2. Indeed I agree... but...

  3. I did consider adding the resetDelay as another parameter of RetrySettings, but then the remaining part I welcome suggestions on how to implement it in a more functional way. My solution uses Async and threadDelay, and thus all the stuff from MonadControl et all is needed. What would be the alternative you would be considering?

Thanks,
Joao

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants