-
Notifications
You must be signed in to change notification settings - Fork 487
Description
The current implementation of dotenv run CLI uses subprocess.Popen, which spawns a child process to execute the specified command.
p = Popen(command,
universal_newlines=True,
bufsize=0,
shell=False,
env=cmd_env)
After spawning the child process, it exits with the same exit code returned by the child process.
ret = run_command(commandline, dotenv_as_dict)
exit(ret)
We can enhance dotenv run usage dramatically while preserving exactly the same behaviour
By switching to os.execvpe instead of subprocess.Popen, we can replace the parent dotenv process with the new process specified by the user. This results in only one active process—the program the user intended to run.
Benefits:
-
No hanging parent process
dotenv runacts as a launcher, so after executingdotenv run redis-server, only the Redis server process remains. The dotenv process, along with its Python interpreter, is completely replaced. This prevents the dotenv process from consuming RAM and other resources, which would otherwise persist until the Redis server exits. -
Proper signal handling
When usingsubprocess.Popen, the parent process (e.g.,dotenv) remains responsible for handling and forwarding signals, which can lead to issues if the command doesn’t receive them directly. For instance, in Docker, if Redis was started withoutexec, it may not get important signals likeSIGTERMwhen the container stops, potentially resulting in improper shutdowns or zombie processes. Usingos.execvpeensures that the command receives signals directly, improving reliability and makingdotenvmore suitable for production environments and improving reliability for DevOps engineers managing containerized applications.
All current logic will be preserved because dotenv run does not do anything special except propagate the child process exit code.
I don't see any downsides of replacing current implementation with exec. Let's exec it :)
If you have any questions or concerns, I would be glad to discuss them.