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.
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
.
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):
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: # ...
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)
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
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): # ...
I'm just going to assume the cmd check_output...
is just a typo, since Python uses def
.
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
cmd
is not a keyword andcmd check_...
is a syntax error. \$\endgroup\$run()
,check_call()
andcheck_output()
. \$\endgroup\$