OpenFn / apollo

GNU Lesser General Public License v2.1
0 stars 2 forks source link

Remove node-calls-python #55

Closed josephjclark closed 6 months ago

josephjclark commented 6 months ago

Sadly, node-calls-python isn't going to be sustainable fit for this project. It's a real shame.

The two issue are: 1) Logging - how to we get python stdout back into node? 2) Concurrency - how do we run multiple modules at once?

Admittedly questions are open about concurrency, but at the very least it gives us problems with logging.

The problem is this:

Possible solutions:

Basically I think that, for now at least, it's going to be just as easy to do child_process.exec and create a new python context each time. At this point I don;'t even care if that's a little slow in production - but I don't even have a reading on HOW slow.

TODO:

josephjclark commented 6 months ago

Another issue coming up is that the python modules have to be DESIGNED to be long lived. You have to think very carefully about what gets instantiated where. Right now the logger has to be created inside main and given a unique id. Module-scoped loggers will forever log to the wrong file. And the same with any clients, you need to think about when it gets instantiated, and be sure you're not accidentally using the wrong client.

This is fairly hard engineering stuff. I'd actually PREFER that each python gets loaded, run and destroyed on every call - it's so much safer.

I suppose we could keep node-calls-python in always-reload mode, but in local dev that feels a little bit unstable (the server keeps dying and I haven't yet diagnosed why), but I also hope that some of the other libraries will make getting the stdout easier.