Add 'channels' options; save cache on action failure; extend docs #45
Conversation
jonashaag
force-pushed
the
cache-failure
branch
from
6f891ff
to
98bc874
Compare
jonashaag
force-pushed
the
cache-failure
branch
2 times, most recently
from
cc5ce1b
to
94117ea
Compare
jonashaag
force-pushed
the
cache-failure
branch
from
94117ea
to
bb59c58
Compare
|
Hah |
jonashaag
force-pushed
the
cache-failure
branch
from
ca51cc0
to
1d61a51
Compare
| @@ -104,11 +104,7 @@ async function installMicromambaPosix (micromambaUrl) { | |||
| linux: 'wget -qO- --retry-connrefused --waitretry=10 -t 5' | |||
| }[MAMBA_PLATFORM] | |||
| const downloadCmd = `${downloadProg} ${micromambaUrl} | tar -xvjO bin/micromamba > ${PATHS.micromambaExe}` | |||
| try { | |||
There was a problem hiding this comment.
Was that download retry there on purpose?
There was a problem hiding this comment.
It would be nice to have a retry of the micromamba download for a couple of times because there can always be some downtime (e.g. on the micromamba server or the anaconda server).
There was a problem hiding this comment.
Added something back.
| @@ -37,24 +37,20 @@ function getInputAsArray (name) { | |||
| .filter(x => x !== '') | |||
| } | |||
|
|
|||
| async function executeBash (command, opts = { setFailed: true }) { | |||
There was a problem hiding this comment.
Changed the execute functions so that they actually halt the action, not only log the error and continue.
| core.info(`${action} env ${envName}`) | ||
| const quotedExtraSpecsStr = selectSelectors(extraSpecs).map(e => `"${e}"`).join(' ') | ||
| const cmd = `micromamba ${action} -n ${envName} ${quotedExtraSpecsStr} --strict-channel-priority -y -f ${envFilePath}` |
There was a problem hiding this comment.
Fixed the code to work with no env file and only extra specs.
| @@ -73,6 +69,10 @@ function sha256 (s) { | |||
| return h.digest().hexSlice() | |||
| } | |||
|
|
|||
| function sha256Short (s) { | |||
There was a problem hiding this comment.
Shortened env file content hashes for better legibility.
Co-authored-by: Tom de Geus <[email protected]>
jonashaag
force-pushed
the
cache-failure
branch
2 times, most recently
from
a4af6e2
to
5af807d
Compare
jonashaag
force-pushed
the
cache-failure
branch
from
64cd512
to
0262775
Compare
jonashaag
changed the title
|
Thanks @jonashaag for this! Would be nice to get merged soon |
|
ping @wolfv |
|
This PR also fixes silently ignored errors in |
|
I like it. I wonder if we're making a mistake by not using YAML lists where appropriate (extra specs and channels could be proper lists...) What do you think? |
|
One argument against it is that it's slightly more cumbersome to have conditional extra-specs/channels: vs (does that even work?) or even I guess we could just ignore empty lists items (is The I guess it's safe to support both syntaxes as well... |
| @@ -7,7 +7,8 @@ inputs: | |||
| environment-file: | |||
| description: >- | |||
| The environment.yml or .lock file for the conda environment. | |||
| If 'false', no enviroment will be created (only Micromamba will be installed). | |||
| If 'false', no enviroment will be created (only Micromamba will be installed) | |||
There was a problem hiding this comment.
I think this might be too late, but could we also use null?
There was a problem hiding this comment.
It seems that GHA deletes inputs with foo: null from the actual inputs that are being passed to the action. foo: 'null' would work but feels ugly. So I suggest to keep false.
|
LGTM! |
|
@wolfv if you're OK with the |
Rel #42
Closes #44