Bogdanp / django_dramatiq

A Django app that integrates with Dramatiq.
https://dramatiq.io
Other
331 stars 77 forks source link

Use subprocess.run() instead of os.execvp() #87

Closed magraeber closed 3 years ago

magraeber commented 3 years ago

I use django_dramatiq in an hybrid cloud setup with Docker worker containers based on Linux and Windows. When running dramatiq workers inside a Windows container with:

python manage.py rundramatiq

I experienced an issue with premature exiting of the main process causing the docker container to shut down immediately after starting. This issue does not pop up on Linux.

Changing from os.execvp() to subprocess.run() in rundramatiq.py perfectly solved this issue. I can now use the same code on both platforms Windows and Linux.

I don't understand too much about these functions, but it seems to me, that subprocess is the newer and recommended way to run external programs in Python. Is there a chance to get this change into django_dramatiq?

Bogdanp commented 3 years ago

Sorry for the late reply, I'd meant to reply here but never got around to it.

There is a significant difference between the exec family of functions on posix systems and running a subprocess: exec* replace the current process with the new one, whereas Python's subprocess just launches a new one. I'd like to preserve the current behavior on posix systems, but I'd be happy to take a PR that conditionally uses subprocess on Windows.

Bogdanp commented 3 years ago

Closed in https://github.com/Bogdanp/django_dramatiq/commit/39efe967cac3514691ab475caf7cdecb1dbaf3cc thanks to @tbock .