Skip to content

Commit

Permalink
Allow opt out of connection file cleanup
Browse files Browse the repository at this point in the history
- Needed for hydrogen since it re-uses the connection file on kernel restart
  • Loading branch information
BenRussert committed Aug 12, 2018
1 parent c036121 commit 9f595a2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
16 changes: 11 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ function writeConnectionFile(portFinderOptions) {
* @param {object} kernelSpec describes a specific
* kernel, see the npm
* package `kernelspecs`
* @param {object} [spawnOptions] `child_process`-like options for [execa]{@link https://github.com/sindresorhus/execa#options}
* @param {object} [spawnOptions] `child_process`-like {@link https://github.com/sindresorhus/execa#options options for execa}
* use `{ cleanupConnectionFile: false }` to disable automatic connection file cleanup
* @return {object} spawnResults
* @return {ChildProcess} spawnResults.spawn spawned process
* @return {string} spawnResults.connectionFile connection file path
Expand All @@ -145,6 +146,7 @@ function launchSpec(kernelSpec, spawnOptions) {
* @param {object} config connection config
* @param {string} connectionFile path to the config file
* @param {object} [spawnOptions] `child_process`-like options for [execa]{@link https://github.com/sindresorhus/execa#options}
* use `{ cleanupConnectionFile: false }` to disable automatic connection file cleanup
* @return {object} spawnResults
* @return {ChildProcess} spawnResults.spawn spawned process
* @return {string} spawnResults.connectionFile connection file path
Expand All @@ -162,21 +164,24 @@ function launchSpecFromConnectionInfo(
);

const defaultSpawnOptions = {
stdio: "ignore"
stdio: "ignore",
cleanupConnectionFile: true
};
const env = Object.assign({}, process.env, kernelSpec.env);
const fullSpawnOptions = Object.assign(
{},
defaultSpawnOptions,
// TODO: see if this interferes with what execa assigns to the env option
{ env: env },
spawnOptions
);

const runningKernel = execa(argv[0], argv.slice(1), fullSpawnOptions);

runningKernel.on("exit", (code, signal) => cleanup(connectionFile));
runningKernel.on("error", (code, signal) => cleanup(connectionFile));

if (fullSpawnOptions.cleanupConnectionFile !== false) {
runningKernel.on("exit", (code, signal) => cleanup(connectionFile));
runningKernel.on("error", (code, signal) => cleanup(connectionFile));
}
return {
spawn: runningKernel,
connectionFile,
Expand All @@ -194,6 +199,7 @@ function launchSpecFromConnectionInfo(
* See the npm package
* `kernelspecs`
* @param {object} [spawnOptions] `child_process`-like options for [execa]{@link https://github.com/sindresorhus/execa#options}
* use `{ cleanupConnectionFile: false }` to disable automatic connection file cleanup
* @return {object} spawnResults
* @return {ChildProcess} spawnResults.spawn spawned process
* @return {string} spawnResults.connectionFile connection file path
Expand Down
28 changes: 27 additions & 1 deletion test/spawn_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,38 @@ const fs = require("fs");
const launch = require("../").launch;
const kernelspecs = require("kernelspecs");

function cleanup(connectionFile) {
// cleanup after our test, fail silently if the test failed
try {
fs.unlinkSync(connectionFile);
} catch (e) {
return;
}
}

describe("launch", () => {
let spawnResult;
let spawnResultNoCleanup;
let kernelName;
it("spawns a kernel", done => {
kernelspecs
.findAll()
.then(kernels => {
const kernel = kernels.python2 || kernels.python3;
return launch(kernel.name);
kernelName = kernel.name;
return launch(kernelName);
})
.then(c => {
spawnResult = c;
expect(c).to.not.be.null;
expect(c.spawn).to.not.be.null;
expect(fs.existsSync(c.connectionFile)).to.be.true;
c.spawn.kill();
return launch(kernelName, { cleanupConnectionFile: false });
})
.then(c => {
spawnResultNoCleanup = c;
spawnResultNoCleanup.spawn.kill();
done();
});
});
Expand All @@ -30,4 +47,13 @@ describe("launch", () => {
done();
}, 100);
});

it("won't clean up connection file if opt out", done => {
const { connectionFile } = spawnResultNoCleanup;
setTimeout(() => {
expect(fs.existsSync(connectionFile)).to.be.true;
cleanup(connectionFile);
done();
}, 100);
});
});

0 comments on commit 9f595a2

Please sign in to comment.