Skip to content

Commit

Permalink
PR: Allow null channels in file to not crash setup.
Browse files Browse the repository at this point in the history
There is an edge case where having `channels` in an environment file
but an empty list, and `channels` missing from the GHA options/keys
returns an unhelpful error message:

`Error: Cannot read property 'join' of null`

I've setup an example here which runs through all the permutations of
`channels` keys being set, missing, and nulled:

https://github.com/Lnaden/ghatesting/runs/1538195898?check_suite_focus=true

This PR attempts to fix that by relaxing the strict equals in `setup.ts`
from

`if (condaConfig["channels"] === "" && channels !== undefined) {`

to

`if (condaConfig["channels"] === "" && channels != null) {`

which should allow `channels != null` to catch both `null` and
`undefined` for `channels`. I also attempted to add an example to the
GitHub Actions tests (Example 9) to test this edge case specifically.

A few comments to this PR:
* I'm not familiar with Typescript and building GHAs, so this might not be correct and I'm happy to make changes and take feedback.
* If you use an env file with an empty `channels` list with the `conda` client to create an environment, it works fine and just defaults to whatever the `conda` is configured to. In my example above, setting no channels in either the file or the GHA keys works fine and pulls from the `default` channel.
* This behavior admittedly is ambiguous, so if you would rather this edge case throw an error, I would like to request a better error message.
  • Loading branch information
Lnaden committed Dec 11, 2020
1 parent 58a291b commit f345689
Show file tree
Hide file tree
Showing 6 changed files with 11,308 additions and 27 deletions.
39 changes: 39 additions & 0 deletions .github/workflows/example-9.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: "Example 9: Empty Channels in file dont crash setup"

on:
pull_request:
branches:
- "*"
push:
branches:
- "master"
schedule:
# Note that cronjobs run on master/main by default
- cron: "0 0 * * *"

jobs:
example-9:
# prevent cronjobs from running on forks
if:
(github.event_name == 'schedule' && github.repository ==
'conda-incubator/setup-miniconda') || (github.event_name != 'schedule')
name: Ex9 (${{ matrix.python-version }}, ${{ matrix.os }})
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: ["ubuntu-latest", "macos-latest", "windows-latest"]
python-version: ["3.7", "2.7"]
steps:
- uses: actions/checkout@v2
- uses: ./
id: setup-miniconda
continue-on-error: true
with:
auto-update-conda: true
python-version: ${{ matrix.python-version }}
environment-file: etc/example-empty-channels-environment.yml
- run: |
conda info
conda list
printenv | sort
5 changes: 3 additions & 2 deletions dist/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22022,7 +22022,8 @@ function setupMiniconda(installerUrl, minicondaVersion, architecture, condaVersi
if (environmentFile) {
let channels;
channels = environmentYaml["channels"];
if (condaConfig["channels"] === "" && channels !== undefined) {
// Check if channels is undefined or null (empty list) in the file. != should catch both, !== will get only one
if (condaConfig["channels"] === "" && channels != null) {
condaConfig["channels"] = channels.join(",");
}
else if (!environmentExplicit) {
Expand Down Expand Up @@ -29298,7 +29299,7 @@ module.exports = defaults;
/* 771 */
/***/ (function(module) {

module.exports = {"_args":[["[email protected]","/Users/goanpeca/Dropbox (Personal)/develop/conda-incubator/setup-miniconda"]],"_from":"[email protected]","_id":"[email protected]","_inBundle":false,"_integrity":"sha512-0td5ijfUPuubwLUu0OBoe98gZj8C/AA+RW3v67GPlGOrvxWjZmBXiBCRU+I8VEiNyJzjth40POfHiz2RB3gImA==","_location":"/cheerio","_phantomChildren":{},"_requested":{"type":"version","registry":true,"raw":"[email protected]","name":"cheerio","escapedName":"cheerio","rawSpec":"1.0.0-rc.3","saveSpec":null,"fetchSpec":"1.0.0-rc.3"},"_requiredBy":["/get-hrefs"],"_resolved":"https://registry.npmjs.org/cheerio/-/cheerio-1.0.0-rc.3.tgz","_spec":"1.0.0-rc.3","_where":"/Users/goanpeca/Dropbox (Personal)/develop/conda-incubator/setup-miniconda","author":{"name":"Matt Mueller","email":"[email protected]","url":"mat.io"},"bugs":{"url":"https://github.com/cheeriojs/cheerio/issues"},"dependencies":{"css-select":"~1.2.0","dom-serializer":"~0.1.1","entities":"~1.1.1","htmlparser2":"^3.9.1","lodash":"^4.15.0","parse5":"^3.0.1"},"description":"Tiny, fast, and elegant implementation of core jQuery designed specifically for the server","devDependencies":{"benchmark":"^2.1.0","coveralls":"^2.11.9","expect.js":"~0.3.1","istanbul":"^0.4.3","jquery":"^3.0.0","jsdom":"^9.2.1","jshint":"^2.9.2","mocha":"^3.1.2","xyz":"~1.1.0"},"engines":{"node":">= 0.6"},"files":["index.js","lib"],"homepage":"https://github.com/cheeriojs/cheerio#readme","keywords":["htmlparser","jquery","selector","scraper","parser","html"],"license":"MIT","main":"./index.js","name":"cheerio","repository":{"type":"git","url":"git://github.com/cheeriojs/cheerio.git"},"scripts":{"test":"make test"},"version":"1.0.0-rc.3"};
module.exports = {"name":"cheerio","version":"1.0.0-rc.3","description":"Tiny, fast, and elegant implementation of core jQuery designed specifically for the server","author":"Matt Mueller <[email protected]> (mat.io)","license":"MIT","keywords":["htmlparser","jquery","selector","scraper","parser","html"],"repository":{"type":"git","url":"git://github.com/cheeriojs/cheerio.git"},"main":"./index.js","files":["index.js","lib"],"engines":{"node":">= 0.6"},"dependencies":{"css-select":"~1.2.0","dom-serializer":"~0.1.1","entities":"~1.1.1","htmlparser2":"^3.9.1","lodash":"^4.15.0","parse5":"^3.0.1"},"devDependencies":{"benchmark":"^2.1.0","coveralls":"^2.11.9","expect.js":"~0.3.1","istanbul":"^0.4.3","jquery":"^3.0.0","jsdom":"^9.2.1","jshint":"^2.9.2","mocha":"^3.1.2","xyz":"~1.1.0"},"scripts":{"test":"make test"}};

/***/ }),
/* 772 */
Expand Down
7 changes: 7 additions & 0 deletions etc/example-empty-channels-environment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name: empty-channels-environment

channels:

dependencies:
- black
- anaconda-client
Loading

0 comments on commit f345689

Please sign in to comment.