Prevent “stop_never” stop condition

[gesslerpd]

This is technically breaking change but doubt it's being used intentionally.

Would like to propose that a ValueError is raised if attempts is None and timeout is None to prevent unbounded stop_never.

This edge-case if required can still be implemented by passing "math.inf" to either parameter.

Think it aligns with goals of "stamina", to reduce bad patterns such as unbounded stop conditions.

I can submit a change but thought I'd raise discussion here first.

Thoughts?

[Hugo-C]

I can see some users wanting to retry forever, and using timeout=None means it is an explicit choice.
But I couldn't find Github code usage in public repo.

[gesslerpd]

Yeah I think it could be desired, was mostly thinking about code that has attempts=None, timeout=variable and then the variable gets set to None somehow, or vice-versa with attempts being set. This check could prevent mistakes like this while still allowing infinity to implement the forever case.

[gesslerpd]

#102 This PR implements this idea

[nmichale]

We use timeout=None and attempts=None to retry forever. It's not public code. We would prefer if that didn't raise an error.

[gesslerpd]

I'd recommend attempts=None, timeout=float('inf') to be more explicit, would work today and its ugly but I'd argue that it should be ugly since it encourages thinking twice about unbounded retry

[hynek]

Hey, sorry everyone for the late response. I've been traveling and to be honest also waffling on the topic.

Clearly, the rationale for #102 is correct.

I would never make this breaking though – the current approach might be a bit primitive, but I think it could be called vaguely Pythonic. stamina is at this point production-grade, so breaking our users like this is at this point not an option anymore.

But I've also got bellyaches when it comes to steering people to use a specific way using warnings to achieve the same thing. It's a bit too much past the "paternalistic" line.

---

I have decided to leave the code as it is, however I have improved the documentation in 133325c.

Feel free to lmk how y'all feel about it.

[gesslerpd]

@hynek Makes sense, I've repurposed #102 to reflect only the "fix" part of that MR if you'd consider reopening that