Fix confusing implementation of try_limit in Promises

[jzoldak]

The __init.py__ method of the bok-choy Promise class has a default value for timeout. This means that when you pass a value for try_limit, it will stop when the first of the limits (tries or time) is reached, which could have been at 30 seconds (the default value) rather than after the number of tries that you had intended. :(

If you only specify try_limit I would assume that your intention is to actually use the value you gave it, not a mysterious default timeout.

We should be careful with a fix tho, because if we change the behavior now it could cause existing tests (not just ours, as edX is not the only org using bok-choy) to fail.

@benpatterson @nedbat @clytwynec Thoughts?

[jzoldak]

@cgoldberg tagging you also for any thoughts/input you might have on this one.

[nedbat]

I'm not sure what the desired semantics were. I can see arguments on both sides, but the doc says, "will poll until either A, B, C", which makes it sound like it was supposed to stop after the default 30 seconds even if the number of tries hadn't been reached. Perhaps the simplest thing to do is to add some clarifying text to the docstring.

[benpatterson]

For me the confusion was in the default timeout of 30s, and how selenium also uses a 30s timeout when trying to instantiate the browser. So I just happened upon a fun and special situation last week since I was working on the Promise that deals with starting the browser itself.

Back to your question though, if we go with an either/or, my concern would be for try_limits that get hung...there would be no default timeout that stops the test.

[jzoldak]

oh, that's right @benpatterson
Now I remember why we did that, so you wouldn't hang yourself accidentally.
@nedbat I like your idea about a better docstring.
I think that's what we should do. I'll put in a PR when I get a chance.
Thanks guys for helping to think this through. 👍

[jzoldak]

Addressed in #167