Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Launch python transformation engine as subprocess of nodejs? #145

Open
joeriexelmans opened this issue Sep 30, 2022 · 2 comments · May be fixed by #146
Open

Launch python transformation engine as subprocess of nodejs? #145

joeriexelmans opened this issue Sep 30, 2022 · 2 comments · May be fixed by #146
Labels
enhancement New feature or request

Comments

@joeriexelmans
Copy link

Hi,

I see that the "launch" script starts both a nodejs process and a python process in parallel. From what I understand, the nodejs process, which is the "main" process, needs the python process, but not the other way around, so wouldn't it be better to launch the python process as a child process from nodejs? In this way, the AToMPM backend can be understood as a single nodejs process, and killing it will also kill the python process. It will also make it unnecessary to have a complex launch script.

Best regards,
Joeri

@BentleyJOakes
Copy link
Member

Hi Joeri. Thanks for the idea! I think that's its a good one, and I'll try to find some time to implement it, unless you want to make a pull request. The only two concerns would be:

@joeriexelmans
Copy link
Author

Hi Bentley,

A very basic (not addressing your concerns), and usable implementation can be achieved by adding the following 2 lines at the end of httpwsd.js:

const childProcess = require('node:child_process');
childProcess.spawn('python', ['mt/main.py']);

About your concerns:

  • Your first concern is valid, and this should indeed be implemented. Output of NodeJS and Python will be interleaved, so maybe we could give Python's output a different color?
  • About your second concern: I agree that the NodeJS script must have the option not to launch the Python child process. For debugging, I imagine it is useful to run NodeJS and Python in different shells.

Anyway, when I look at the source code of the Python script, I don't see any flags that can be set. So the only remaining concern is selecting a specific Python version. This can also be achieved by passing a modified PATH environment variable to NodeJS, containing the right python version.

If you agree with these points, I'm willing to implement this feature and create a pull request.

@joeriexelmans joeriexelmans linked a pull request Oct 7, 2022 that will close this issue
@BentleyJOakes BentleyJOakes added the enhancement New feature or request label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

2 participants