Add test.support.busy_retry()

[vstinner]

Add busy_wait() and increasing_sleep() functions to test.support.

[vstinner]

cc @erlend-aasland @corona10 @serhiy-storchaka @methane

[vstinner]

Tests / Check if generated files are up to date (pull_request) Failing after 49s

"Configure ccache action" failed with:

"Error: reserveCache failed: Cache service responded with 503"

[vstinner]

"Configure ccache action" failed with: "Error: reserveCache failed: Cache service responded with 503"

actions/cache#820

[vstinner]

When writing this PR, I saw code that I wanted to change, but I tried to minimize changes of this PR :-D I will write a follow-up PR to enhance tests.

[serhiy-storchaka]

Nice. But some new code is not equivalent to the old code (in particularly related to signal handling). In some cases the old code used very short sleep or no sleep, or other operation instead of sleep. I am not sure that replacements preserve the intention of tests.

[vstinner]

But some new code is not equivalent to the old code (in particularly related to signal handling).

Oh sorry, I reverted these changes. Are remaining changes correct?

[vstinner]

I dislike increasing_sleep() name. What do you think of busy_wait_and_sleep()?

[vstinner]

The "busy wait" term seems commonly used: https://en.wikipedia.org/wiki/Busy_waiting

[serhiy-storchaka]

I dislike increasing_sleep() name. What do you think of busy_wait_and_sleep()?

You can merge both functions if skip time.sleep() when init_delay is 0.

I though also about alternative interface. Instead of

for _ in support.increasing_sleep(support.SHORT_TIMEOUT):
    if check():
        break

you could write write

support.wait_for(check, timeout=support.SHORT_TIMEOUT)

(looks like Condition.wait_for())
or

w = support.expired(support.SHORT_TIMEOUT)
while not check() and not expired:
    pass

(it is closer to the original code).

But I do not want to start bikeshedding. It is all wild fantasy. These functions are for internal use. The current code of the PR is good enough.

[methane]

bikeshedding: busy_retry and sleeping_retry.

[vstinner]

support.wait_for(check, timeout=support.SHORT_TIMEOUT)

First, I wanted to do something like that, but I dislike defining nested functions or lambda functions. If you read the PR, you can see that existing tests look the same with my change. There is no indirection (function call), the code remains "flat".

[vstinner]

increasing_sleep() is a bad name. The delay is not "increasing", it's exponential. What the function does is more an: https://en.wikipedia.org/wiki/Exponential_backoff

IMO tenacity API https://tenacity.readthedocs.io/en/latest/api.html#wait-functions should be a good inspiration for function names. I renamed increasing_sleep() to wait_exponential().

[serhiy-storchaka]

bikeshedding: busy_retry and sleeping_retry.

Yes, I would expect the word "repeat" in the name too. But I do not want to make it a requirement.

First, I wanted to do something like that, but I dislike defining nested functions or lambda functions. If you read the PR, you can see that existing tests look the same with my change. There is no indirection (function call), the code remains "flat".

I understood you. While in some cases it is easy to write a lambda, in other cases, when you use several statement in the loop or even try/except block, the local function is unavoidable, and it makes the code less straight.

increasing_sleep() is a bad name. The delay is not "increasing", it's exponential.

I think that the name should describe the purpose of the function, not the implementation detail. In some cases linear grow of delays (and quadratic of total time) may be more preferable than exponential grow.

What do you think about merging two functions into a single function?

[vstinner]

IMO the two functions are too different and deserve to have different names. In terms of API, currently, I propose:

• busy_wait(timeout)
• wait_exponential(timeout) -- was called increasing_sleep(timeout) in the first version

@methane proposed:

• busy_retry(timeout)
• sleeping_retry(timeout)

tenacity API seems to be:

• stop_after_delay(timeout)
• wait_exponential(max=timeout)

I like to put "sleep" in the name, so it's easy to understand that the function calls time.sleep(). I have no preference between "retry and "wait". Maybe retry better highlights that it's a loop and it calls the loop body multiple times until it "succeed" (somehow).

In short, I like names proposed by @methane. @serhiy-storchaka: do you have a preference?

By the way, the whole API is:

wait_exponential(timeout, err_msg=None, /, *, init_delay=0.001, max_delay=1.0, error=True)

Are you fine with init_delay and max_delay parameter names? If not, do you have a suggestion?

Well, I'm not sure that the API matters so much, it's a test module :-D It must not be used outside Python ;-)

[serhiy-storchaka]

I like @methane proposal.

[vstinner]

Thanks @serhiy-storchaka and @methane for your reviews :-) I merged my PR. IMO the tests are now easier to read with the changes. Also, they might be faster (if the condition becomes true quickly) and uses less CPU (lower system load) if the condition takes seconds to complete (sleep longer).

[bedevere-app]

GH-110341 is a backport of this pull request to the 3.11 branch.