-
Notifications
You must be signed in to change notification settings - Fork 69
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
stdlib.run: Fix problems when an executable is missing
Currently, if the executable required in `run` does not exist (or is not executable), the child process's code is not replaced by `os.execvpe` function and it raises the OSError instead. However, the parent process does not get this OSError. It consumes exit code, stderr, ... of the child process. So in case the code does something like this: ``` try: result = run(['non-executable']) except OSError: pass except CalledProcessError: # do something.. ``` the child process passes and do whatever.. In case it ends with zero exit code, the obtained result is usually something totally different than expected in actors. Also there could be problems with non-idempotent code, when some actions could be done twice (once executed by the child, send time executed by the parent [current] process). We have realized that number of existing leapp actors for in-place upgrades already count with the raise of OSError when executable cannot be used. So we choosed for now to check whether executables are present and raise OSError if not, so we are sure that only one process leave the function really. Also applied another seatbelt into the child process - if the OSError is raised anyway despite our checks (e.g. SELinux prevents the execution) let's just kill the process instead giving it a possibility to continue. In such a case, always print a msg to stderr of the child process and exit with ecode 1. Note: To check an executable we use `distutils.spawn.find_executable` which is deprecated in Python 3 and will be dropped in Python 3.12. However it exists now for Python 2 & 3, so we use this one for now and will replace it in future by `shutils.which` when the time comes. Additional changes: * Make pylint happy (set noqa for E721 for the check the Fields is not created directly). The "isinstance" function has a different behaviour in this case than we want. * Fix imports to make pylint happy * Set `result` to empty dict instead of None: in case a ValueError or TypeError is raised inside the _call function, it's expected to copy the `result` content inside the final block, however in case the results is None, it fails and raise additional exception that covers the original one. * Update unit tests to cover issues with missing executable.
- Loading branch information
Showing
4 changed files
with
75 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters