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

Allow setting detached #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow setting detached #29

wants to merge 1 commit into from

Conversation

mifi
Copy link

@mifi mifi commented Dec 22, 2019

Allow setting detached: true on the child process so that we can prevent it from propagating signals from parent

@munro
Copy link
Member

munro commented Dec 24, 2019

It looks like the Travis CI broke support building for Python 2.6, 3.2, 3.3, annoying... I think I have to manually specify the Ubuntu distribution for these Python versions

travis-ci/travis-ci#9133 (comment)

@munro
Copy link
Member

munro commented Apr 27, 2020

@mifi I still haven't dug into the travis-ci error, so any help to get this moving is greatly appreciated! Also if you could write an automated test that uses detached, that would be awesome—really so we can make sure this feature always works for you in the future.

I think the integration test could be done pretty easily, here's some pseudo code:

const isRunning = require('is-running');

async function test() {
    // to do an automated test for this, we need to run our JS code in another process
    // so that when that process exits, we can test that the Python process is still running.
    const processPythonPid = await someFunctionThatSpawnAProcess(async () => {
        const pythonBridge = require('./');
        const python = pythonBridge({detached: true});
        await python.exec`import os`;
        const pythonPid = python`os.getpid()`;
        t.equal(pythonPid, python.pid, 'same pid');

        python.ps.unref();

        return python.ps.pid;
    });

    t.assertTrue(await isRunning(processPythonPid), 'python process is still running after nodejs terminated');

    process.kill(processPythonPid, 'SIGKILL');
}

function someFunctionThatSpawnAProcess(f) {
    // @TODO do we have to write this, or is there an NPM module for this?
}

Alternatively, you could launch a detached process for background work from the Python side, which wouldn't require any changes to the python-bridge API.

Anyway, I'm pretty easy going, so I'd love to know your thoughts, or if you found another solution.

@mdesmet
Copy link

mdesmet commented Oct 26, 2022

I don't think that test would be easy. We could spawn another process through node -e "my code here" but there wouldn't be any way for the processes to communicate, eg getting a reference to any variable from the other process. This is actually the whole point about node being single threaded.

Not sure I follow the alternative solution regarding the Python background process. The Python process is launched by the exact spawn that needs to be detached for Windows to not show a console window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants