Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ Workbench
* Bootstrap and React Bootstrap dependencies have been updated to their latest
stable versions (Bootstrap 5.3.8, React Bootstrap 2.10.10).
(`#1798 <https://github.com/natcap/invest/issues/1798>`_)
* Added an option in the Manage Plugins modal to configure which
conda executable is used to run plugins.
(`#2645 <https://github.com/natcap/invest/issues/2645>`_)
* Added an option in the Manage Plugins modal to configure which
conda environment is used to run each plugin.
(`#2646 <https://github.com/natcap/invest/issues/2646>`_)
* React dependency has been updated to its latest stable version (19.2.7).
(`#2481 <https://github.com/natcap/invest/issues/2481>`_)

Expand Down
1 change: 1 addition & 0 deletions workbench/src/main/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export const createWindow = async () => {

settingsStore.set('investExe', findInvestBinaries(ELECTRON_DEV_MODE));
settingsStore.set('micromamba', findMicromambaExecutable(ELECTRON_DEV_MODE));
settingsStore.set('defaultMicromamba', findMicromambaExecutable(ELECTRON_DEV_MODE));
// No plugin server processes should persist between workbench sessions
// In case any were left behind, remove them
const plugins = settingsStore.get('plugins');
Expand Down
5 changes: 3 additions & 2 deletions workbench/src/main/setupAddRemovePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async function createBaseEnv(baseEnvPrefix, micromamba) {
logger.info(`creating base environment from file:\n${baseEnvYMLContents}`);
fs.writeFileSync(baseEnvYMLPath, baseEnvYMLContents);
await spawnWithLogging(micromamba, [
'create', '--yes', '--prefix', `"${baseEnvPrefix}"`,
'env', 'create', '--yes', '--prefix', `"${baseEnvPrefix}"`,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It seems that "env create" rather than just "create" is necessary for compatibility with conda.

'--file', `"${baseEnvYMLPath}"`]);
}

Expand Down Expand Up @@ -158,7 +158,7 @@ async function installPlugin(
// conda-forge build, setup.py requires this env variable.
process.env['NATCAP_INVEST_GDAL_LIB_PATH'] = `${pluginEnvPrefix}/Library`;
await spawnWithLogging(micromamba, [
'create', '--yes', '--prefix', `"${pluginEnvPrefix}"`,
'env', 'create', '--yes', '--prefix', `"${pluginEnvPrefix}"`,
'--file', `"${pluginEnvYMLPath}"`]);
logger.info('created micromamba env for plugin');
event.sender.send('plugin-install-status', i18n.t('Installing plugin into environment...'));
Expand Down Expand Up @@ -210,6 +210,7 @@ function storePluginMetadataSync(
modelTitle: modelTitle,
type: 'plugin',
source: installString,
defaultEnv: pluginEnvPrefix,
env: pluginEnvPrefix,
version: version,
}
Expand Down
150 changes: 148 additions & 2 deletions workbench/src/renderer/components/PluginModal/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export default function PluginModal(props) {
const [url, setURL] = useState('');
const [revision, setRevision] = useState('');
const [path, setPath] = useState('');
const [condaPath, setCondaPath] = useState('');
const [pluginEnvs, setPluginEnvs] = useState({});
const [installErr, setInstallErr] = useState('');
const [uninstallErr, setUninstallErr] = useState('');
const [pluginToRemove, setPluginToRemove] = useState('');
Expand Down Expand Up @@ -62,6 +64,21 @@ export default function PluginModal(props) {
setPluginSourceMissingError(false);
};

useEffect(() => {
ipcRenderer.invoke(
ipcMainChannels.GET_SETTING, 'micromamba'
).then((data) => setCondaPath(data));
ipcRenderer.invoke(
ipcMainChannels.GET_SETTING, 'plugins'
).then((data) => setPluginEnvs(
Object.fromEntries(
Object.keys(data).map(
(pluginID) => [pluginID, data[pluginID].env]
)
)
))
}, []);

useEffect(() => {
clearFormErrors();
}, [installFrom]);
Expand Down Expand Up @@ -149,6 +166,36 @@ export default function PluginModal(props) {
);
};

const resetCondaPath = () => {
ipcRenderer.invoke(
ipcMainChannels.GET_SETTING, 'defaultMicromamba'
).then((data) => {
setCondaPath(data);
});
};

const saveCondaPath = () => {
ipcRenderer.send(
ipcMainChannels.SET_SETTING, 'micromamba', condaPath
);
};

const resetPluginEnv = (pluginID) => {
ipcRenderer.invoke(
ipcMainChannels.GET_SETTING, `plugins.${pluginID}.defaultEnv`
).then((value) => {
setPluginEnvs({...pluginEnvs, [pluginID]: value});
});
};

const savePluginEnvs = () => {
Object.entries(pluginEnvs).forEach(([pluginID, envPath]) => {
ipcRenderer.send(
ipcMainChannels.SET_SETTING, `plugins.${pluginID}.env`, envPath
);
});
};

const selectDirectory = async (event) => {
const data = await ipcRenderer.invoke(
ipcMainChannels.SHOW_OPEN_DIALOG, { properties: ['openDirectory'] }
Expand All @@ -157,7 +204,7 @@ export default function PluginModal(props) {
// dialog defaults allow only 1 selection
setPath(data.filePaths[0]);
}
}
};

useEffect(() => {
ipcRenderer.on('plugin-install-status', (msg) => { setStatusMessage(msg); });
Expand Down Expand Up @@ -187,7 +234,6 @@ export default function PluginModal(props) {
let pluginFields;
if (installFrom === 'url') {
pluginFields = (

<Row>
<Form.Group as={Col} xs={7}>
<Form.Label htmlFor="url">{t('Git URL')}</Form.Label>
Expand Down Expand Up @@ -433,6 +479,106 @@ export default function PluginModal(props) {
}
</div>
</Form>
<hr />
<Form aria-labelledby="configure-conda-form-title" aria-describedby="conda-executable-description">
<Form.Group>
<h5 id="configure-conda-form-title" className="mb-3">{t('Configure conda executable (Advanced)')}</h5>
<Form.Text
as="span"
id="conda-executable-description"
className="plugin-form-text mb-3"
>
{t('InVEST is distributed with a copy of micromamba, a conda-like '
+ 'package manager that is used to manage plugin environments. '
+ 'If you have conda or mamba installed elsewhere on the system, '
+ 'you can configure InVEST to use that executable instead. This '
+ 'may be useful if you run into limitations of the included '
+ 'micromamba distribution.')}
</Form.Text>
<Form.Label htmlFor="condaPath">{t('Conda or mamba executable')}</Form.Label>
<div className="d-flex flex-nowrap w-100">
<Form.Control
id="condaPath"
type="text"
value={condaPath}
onChange={(event) => setCondaPath(event.target.value)}
className="me-1"
/>
<Button
aria-label="browse for conda executable"
className="browse-button ms-1 me-1"
id="browse-conda-button"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for an id here unless you are referencing it elsewhere (and it doesn't look like you are). If you want a handle for testing, it can be convenient to add a data- attribute that is clearly there for testing purposes. (For example, data-testid, which we use in a couple of components, automatically works with the DOM Testing Library's findByTestId query.)

variant="outline-dark"
onClick={selectDirectory}
>
<MdFolderOpen />
</Button>
<Button onClick={resetCondaPath} className="text-nowrap ms-1">
{t('Reset')}
</Button>
</div>
<Button onClick={saveCondaPath} className="text-nowrap mt-3">
{t('Save')}
</Button>
</Form.Group>
</Form>
<hr />
<Form aria-labelledby="configure-plugin-envs-form-title" aria-describedby="plugin-env-description">
<Form.Group>
<h5 id="configure-plugin-envs-form-title" className="mb-3">{t('Configure plugin environments (Advanced)')}</h5>
<Form.Text
as="span"
id="plugin-env-description"
className="plugin-form-text mb-3"
>
{t('InVEST creates a separate conda environment for each installed '
+ 'plugin. You may override this and provide a path to a different '
+ 'conda environment, which may be useful for development and '
+ 'debugging.')}
</Form.Text>
{Object.keys(plugins).map((pluginID) => (
<Form.Group key={`${pluginID}-env-group`}>
<Form.Label htmlFor={pluginID}>
{pluginID}
</Form.Label>
<div
className="d-flex flex-nowrap w-100 mb-1"
>
<Form.Control
id={pluginID}
type="text"
value={pluginEnvs[pluginID]}
onChange={(event) => setPluginEnvs(
{...pluginEnvs, [pluginID]: event.target.value}
)}
className="me-1"
/>
<Button
aria-label="browse for env"
className="browse-button ms-1 me-2"
id="browse-env-button"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for an id here either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

…unless that id ends up being referenced by the click handler, in which case, make sure each instance of this button has a unique id (e.g., including the relevant plugin ID).

variant="outline-dark"
onClick={selectDirectory}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I couldn't figure out why the browse buttons didn't seem to be working for me, until I looked more closely at the code. Every browse button in the modal is calling selectDirectory on click… and selectDirectory always populates the path field in the 'Add a plugin' form! So when I use a browse button to choose a conda path or a plugin env, my selection lands in the wrong form. You may want to write separate functions, or it might make sense to refactor selectDirectory to take another argument or perhaps parse data attached to the event.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some automated tests might be helpful here, too! 😃

>
<MdFolderOpen />
</Button>
<Button
onClick={() => resetPluginEnv(pluginID)}
className="text-nowrap"
>
{t('Reset')}
</Button>
</div>
</Form.Group>
))}
<Button
onClick={savePluginEnvs}
className="text-nowrap mt-3"
disabled={!Object.keys(pluginEnvs).length}>
{t('Save')}
</Button>
</Form.Group>
</Form>
</Modal.Body>
);
if (installErr) {
Expand Down
2 changes: 1 addition & 1 deletion workbench/src/renderer/styles/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ input[type=text]::placeholder {

/* Manage Plugins modal */
.plugin-modal {
width: 34rem;
width: 40rem;
form {
& > div,
& > button {
Expand Down
8 changes: 7 additions & 1 deletion workbench/tests/renderer/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,15 @@ describe('Main menu interactions', () => {
});

test('Open Plugins Modal from menu', async () => {
ipcRenderer.invoke.mockImplementation((channel) => {
ipcRenderer.invoke.mockImplementation((channel, arg) => {
if (channel === ipcMainChannels.HAS_MSVC) {
return Promise.resolve(true);
} else if (channel === ipcMainChannels.GET_SETTING) {
if (arg === 'micromamba') {
return Promise.resolve('micromamba');
} else if (arg === 'plugins') {
return Promise.resolve({});
}
}
return Promise.resolve();
});
Expand Down
12 changes: 11 additions & 1 deletion workbench/tests/renderer/plugin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe('Add plugin modal', () => {

test('Add a plugin: failure with error displayed', async () => {
const errorString = 'Failed to clone repository.';
const spy = ipcRenderer.invoke.mockImplementation((channel) => {
const spy = ipcRenderer.invoke.mockImplementation((channel, setting) => {
if (channel === ipcMainChannels.HAS_MSVC) {
return Promise.resolve(true);
}
Expand All @@ -195,6 +195,16 @@ describe('Add plugin modal', () => {
new Error(errorString)
);
}
if (channel === ipcMainChannels.GET_SETTING) {
if (setting === 'plugins') {
return Promise.resolve({
foo: {
modelTitle: 'Foo',
type: 'plugin',
},
});
}
}
return Promise.resolve();
});
const {
Expand Down
Loading