Python retry logic based on return code and output

Question

I am trying to write a function which execute the given command with given number of retry and delay. If the return code is 0 and output string have expected_output(partial) have to break the loop and return the return code and output. Is there any better to write this code?

cmd check_output_and_retry(cmd, expected_output, delay=1, retry=3):
    for i in range(retry):
        # Run the given command.
        # Return value will be return code with cmd output.
        ret, output = execute_cmd(cmd)
        if ret == 0 and expected_out in output:
            break
        else:
            time.sleep(delay)
    return ret, output

Answer 1

Without getting into a code golf'esque "shortest possible way to write this function", I'll just note a few alternatives to your particular implementation, and you can consider if you think they're simpler or not.

1. if ret == 0 is equivalent to if not ret. While that's probably not more legible in the if statement, it might be if you named this variable, e.g. program_failed = not ret.

2. expected_out in output is just one thing someone might want to do to evaluate if the output was what they wanted (e.g., maybe instead they want to do a regex check). You may want to replace this with a totally freeform function, then just call that function and use its result; e.g., if ret == 0 and output_ok(output):

3. Building on both of these, it might be prettier to name the conditions:

   status_ok = ret == 0
   output_ok = check_output(output)
   if status_ok and output_ok:  # ...
   

4. Due to the nature of the if statement, the else condition is superfluous. It'd be equivalent to write:

   if condition:
       break
   
   time.sleep(delay)
   

5. You return ret, output whether or not the program succeeds, which may be confusing. Since your function's purpose is to check the output of the program and retry if it fails, it seems odd to return "successfully" if the program fails those checks just because it's run out of retries. Consider raising an exception instead.

   for _ in range(retry):
       # ...
       if condition:
           break
       # ...
   else:
       raise ValueError("Failed to run program successfully, <some useful information about ret, output, etc.>")
   return output  # Only output, since ``ret`` is guaranteed to be 0
   

6. Pycharm and potentially other Python stylers might get upset at you for having a named variable you don't use: i. Consider replacing with _:

   for _ in range(retry):  # ...
   

7. I'm just going to assume the cmd check_output... is just a typo, since Python uses def.

8. Write a docstring!

Again, you might not think all of the above are improvements, just alternatives. If you did follow them all, you might end up with the following:

def check_output_and_retry(cmd, check_output: lambda out: True, delay=1, retry=3):
    """Runs command until it succeeds. Raises a ValueError if it can't get the program to succeed.

    Args:
        cmd (str): The command to run
        check_output (callable): A function that takes the output string and returns whether or not it's ok
        delay (int|float): The number of seconds to wait between attempts
        retry (int): The number of times to try running the command before failing

    Returns:
        str: The program output. The program return code is guaranteed to be 0.
    """
    for _ in range(retry):
        # Run the given command.
        ret, output = execute_cmd(cmd)
        status_ok = ret == 0
        output_ok = check_output(output)
        if status_ok and output_ok:
            break

        time.sleep(delay)
    else:
        raise ValueError(f"Program failed to execute within {retry} attempts (status: {ret}: {output}")

    # Return value will be cmd output, since the return code is guaranteed to always be 0
    return output