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

Log stdout and stderr from plugin installation and removal #1682

Merged
merged 8 commits into from
Nov 13, 2024
Merged
148 changes: 99 additions & 49 deletions workbench/src/main/setupAddRemovePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import upath from 'upath';
import fs from 'fs';
import { tmpdir } from 'os';
import toml from 'toml';
import { execSync } from 'child_process';
import { execSync, spawn } from 'child_process';
import { ipcMain } from 'electron';

import { getLogger } from './logger';
Expand All @@ -11,59 +11,112 @@ import { settingsStore } from './settingsStore';

const logger = getLogger(__filename.split('/').slice(-1)[0]);

/**
* Spawn a child process and log its stdout, stderr, and any error in spawning.
*
* child_process.spawn is called with the provided cmd, args, and options,
* and the windowsHide option set to true.
*
* Required properties missing from the store are initialized with defaults.
* Invalid properties are reset to defaults.
* @param {string} cmd - command to pass to spawn
* @param {Array} args - command arguments to pass to spawn
* @param {object} options - options to pass to spawn.
* @returns {Promise} resolves when the command finishes with exit code 0.
* Rejects with error otherwise.
*/
function spawnWithLogging(cmd, args, options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this function have a docstring above it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a docstring

logger.info(cmd, args);
const cmdProcess = spawn(cmd, args, { ...options, windowsHide: true });
if (cmdProcess.stdout) {
cmdProcess.stderr.on('data', (data) => logger.info(data.toString()));
cmdProcess.stdout.on('data', (data) => logger.info(data.toString()));
}
return new Promise((resolve, reject) => {
cmdProcess.on('error', (err) => {
logger.error(err);
reject(err);
});
cmdProcess.on('close', (code) => {
if (code === 0) {
resolve(code);
} else {
reject(code);
}
});
});
}

export function setupAddPlugin() {
ipcMain.handle(
ipcMainChannels.ADD_PLUGIN,
(e, pluginURL) => {
async (e, pluginURL) => {
logger.info('adding plugin at', pluginURL);

const mamba = settingsStore.get('mamba');
let envName;
let pluginID;
let pluginName;
let pluginPyName;

try {
// Create a temporary directory and check out the plugin's pyproject.toml
const tmpPluginDir = fs.mkdtempSync(upath.join(tmpdir(), 'natcap-invest-'));
execSync(
`git clone --depth 1 --no-checkout ${pluginURL} "${tmpPluginDir}"`,
{ stdio: 'inherit', windowsHide: true }
);
execSync('git checkout HEAD pyproject.toml', { cwd: tmpPluginDir, stdio: 'inherit', windowsHide: true });
await spawnWithLogging(
'git',
['clone', '--depth', '1', '--no-checkout', pluginURL, tmpPluginDir]
).then(async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emlys , forgive me for popping in here. I don't think we need the .then chaining now that we're using await and resolving the promise when the spawn process closes. I think you should just be able to do like this,

await spawnWithLoggin('git clone ...');
await spawnWithLogging('git checkout ...');
...

await spawnWithLogging(
'git',
['checkout', 'HEAD', 'pyproject.toml'],
{ cwd: tmpPluginDir }
);
}).then(async () => {
// Read in the plugin's pyproject.toml, then delete it
const pyprojectTOML = toml.parse(fs.readFileSync(
upath.join(tmpPluginDir, 'pyproject.toml')
).toString());
fs.rmSync(tmpPluginDir, { recursive: true, force: true });

// Read in the plugin's pyproject.toml, then delete it
const pyprojectTOML = toml.parse(fs.readFileSync(
upath.join(tmpPluginDir, 'pyproject.toml')
).toString());
fs.rmSync(tmpPluginDir, { recursive: true, force: true });

// Access plugin metadata from the pyproject.toml
const pluginID = pyprojectTOML.tool.natcap.invest.model_id;
const pluginName = pyprojectTOML.tool.natcap.invest.model_name;
const pluginPyName = pyprojectTOML.tool.natcap.invest.pyname;

// Create a conda env containing the plugin and its dependencies
const envName = `invest_plugin_${pluginID}`;
const mamba = settingsStore.get('mamba');
execSync(`${mamba} create --yes --name ${envName} -c conda-forge "python<3.12" "gdal<3.6"`,
{ stdio: 'inherit', windowsHide: true });
logger.info('created mamba env for plugin');
execSync(`${mamba} run --name ${envName} pip install "git+${pluginURL}"`,
{ stdio: 'inherit', windowsHide: true });
logger.info('installed plugin into its env');
// Access plugin metadata from the pyproject.toml
pluginID = pyprojectTOML.tool.natcap.invest.model_id;
pluginName = pyprojectTOML.tool.natcap.invest.model_name;
pluginPyName = pyprojectTOML.tool.natcap.invest.pyname;

// Write plugin metadata to the workbench's config.json
const envInfo = execSync(`${mamba} info --envs`, { windowsHide: true }).toString();
logger.info(`env info:\n${envInfo}`);
const envPath = envInfo.match(`${envName}\\s+(.+)$`)[1];
logger.info(`env path:\n${envPath}`);
logger.info('writing plugin info to settings store');
settingsStore.set(
`plugins.${pluginID}`,
{
model_name: pluginName,
pyname: pluginPyName,
type: 'plugin',
source: pluginURL,
env: envPath,
}
);
logger.info('successfully added plugin');
// Create a conda env containing the plugin and its dependencies
envName = `invest_plugin_${pluginID}`;
await spawnWithLogging(
mamba,
['create', '--yes', '--name', envName, '-c', 'conda-forge', '"python<3.12"', '"gdal<3.6"']
);
logger.info('created mamba env for plugin');
}).then(async () => {
await spawnWithLogging(
mamba,
['run', '--verbose', '--no-capture-output', '--name', envName, 'pip', 'install', `git+${pluginURL}`]
);
logger.info('installed plugin into its env');
})
.then(() => {
// Write plugin metadata to the workbench's config.json
const envInfo = execSync(`${mamba} info --envs`, { windowsHide: true }).toString();
logger.info(`env info:\n${envInfo}`);
const regex = new RegExp(String.raw`^${envName} +(.+)$`, 'm');
const envPath = envInfo.match(regex)[1];
logger.info(`env path:\n${envPath}`);
logger.info('writing plugin info to settings store');
settingsStore.set(
`plugins.${pluginID}`,
{
model_name: pluginName,
pyname: pluginPyName,
type: 'plugin',
source: pluginURL,
env: envPath,
}
);
logger.info('successfully added plugin');
});
} catch (error) {
return error;
}
Expand All @@ -74,16 +127,13 @@ export function setupAddPlugin() {
export function setupRemovePlugin() {
ipcMain.handle(
ipcMainChannels.REMOVE_PLUGIN,
(e, pluginID) => {
async (e, pluginID) => {
logger.info('removing plugin', pluginID);
try {
// Delete the plugin's conda env
const env = settingsStore.get(`plugins.${pluginID}.env`);
const mamba = settingsStore.get('mamba');
execSync(
`${mamba} remove --yes --prefix "${env}" --all`,
{ stdio: 'inherit' }
);
await spawnWithLogging(mamba, ['remove', '--yes', '--prefix', `"${env}"`, '--all']);
// Delete the plugin's data from storage
settingsStore.delete(`plugins.${pluginID}`);
logger.info('successfully removed plugin');
Expand Down
7 changes: 2 additions & 5 deletions workbench/src/renderer/components/PluginModal/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function PluginModal(props) {
setLoading(false);
updateInvestList();
if (addPluginErr) {
setErr(addPluginErr);
setErr(true);
} else {
setShowPluginModal(false);
}
Expand Down Expand Up @@ -119,10 +119,7 @@ export default function PluginModal(props) {
if (err) {
modalBody = (
<Modal.Body>
<span>{t('Plugin installation failed')}</span>
<br />
<br />
<span>{err.toString()}</span>
{t('Plugin installation failed. Check the workbench log for details.')}
</Modal.Body>
);
}
Expand Down
Loading