-
Notifications
You must be signed in to change notification settings - Fork 76
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
Distribute micromamba executable for plugins #1707
Changes from 11 commits
127d887
897fd85
5e08b03
d864705
b67af89
21275e2
f35ad8e
1461bbe
7652d63
d93a4ce
df213fb
993944d
dbd6419
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -36,8 +36,12 @@ const config = { | |||||
to: 'invest', | ||||||
}, | ||||||
{ | ||||||
from: '../dist/Miniforge3', | ||||||
to: 'Miniforge3', | ||||||
from: '../dist/micromamba', // mac | ||||||
to: 'micromamba', | ||||||
}, | ||||||
{ | ||||||
from: '../dist/micromamba.exe', // windows | ||||||
to: 'micromamba', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this in another commit before I saw this |
||||||
}, | ||||||
{ | ||||||
from: '../dist/userguide', | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ export function setupAddPlugin() { | |
async (e, pluginURL) => { | ||
try { | ||
logger.info('adding plugin at', pluginURL); | ||
const mamba = settingsStore.get('mamba'); | ||
const micromamba = settingsStore.get('micromamba'); | ||
// Create a temporary directory and check out the plugin's pyproject.toml | ||
const tmpPluginDir = fs.mkdtempSync(upath.join(tmpdir(), 'natcap-invest-')); | ||
await spawnWithLogging( | ||
|
@@ -81,19 +81,19 @@ export function setupAddPlugin() { | |
// Create a conda env containing the plugin and its dependencies | ||
const envName = `invest_plugin_${pluginID}`; | ||
await spawnWithLogging( | ||
mamba, | ||
micromamba, | ||
['create', '--yes', '--name', envName, '-c', 'conda-forge', '"python<3.12"', '"gdal<3.6"'] | ||
); | ||
logger.info('created mamba env for plugin'); | ||
logger.info('created micromamba env for plugin'); | ||
await spawnWithLogging( | ||
mamba, | ||
['run', '--verbose', '--no-capture-output', '--name', envName, 'pip', 'install', `git+${pluginURL}`] | ||
micromamba, | ||
['run', '--name', envName, 'pip', 'install', `git+${pluginURL}`] | ||
); | ||
logger.info('installed plugin into its env'); | ||
// Write plugin metadata to the workbench's config.json | ||
const envInfo = execSync(`${mamba} info --envs`, { windowsHide: true }).toString(); | ||
const envInfo = execSync(`${micromamba} info --name ${envName}`, { windowsHide: true }).toString(); | ||
logger.info(`env info:\n${envInfo}`); | ||
const regex = new RegExp(String.raw`^${envName} +(.+)$`, 'm'); | ||
const regex = /env location : (.+)/; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I've seen this syntax before. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's a regex literal. My linter recommended this over a string. |
||
const envPath = envInfo.match(regex)[1]; | ||
logger.info(`env path:\n${envPath}`); | ||
logger.info('writing plugin info to settings store'); | ||
|
@@ -123,8 +123,8 @@ export function setupRemovePlugin() { | |
try { | ||
// Delete the plugin's conda env | ||
const env = settingsStore.get(`plugins.${pluginID}.env`); | ||
const mamba = settingsStore.get('mamba'); | ||
await spawnWithLogging(mamba, ['remove', '--yes', '--prefix', `"${env}"`, '--all']); | ||
const micromamba = settingsStore.get('micromamba'); | ||
await spawnWithLogging(micromamba, ['remove', '--yes', '--prefix', `"${env}"`, '--all']); | ||
// Delete the plugin's data from storage | ||
settingsStore.delete(`plugins.${pluginID}`); | ||
logger.info('successfully removed plugin'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just to make sure the command works, and if it doesn't we'll fail / exit here? If so, it might be nice to have a short comment here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just testing the executable. Which might not be necessary any more, but I think it's fine to leave in. Added a comment