Fix cml runner --cloud-gpu to accept nogpu

[dacbd]

• Closes CML runner error parsing cloud-gpu #746

[dacbd]

Functional test:

dabarnes@ubuntu:~/projects/cml$ ./bin/cml.js runner \
>     --single \
>     --log=debug \
>     --idle-timeout=1800 \
>     --token=xxx \
>     --cloud=gcp \
>     --cloud-region=us-west \
>     --cloud-type=m \
>     --cloud-gpu=nogpu \
>     --repo=xxx \
>     --driver=github \
>     --cloud-hdd-size=10
info: Preparing workdir /home/dabarnes/.cml/cml-2g7sdfdtiy...
info: Deploying cloud runner plan...
info: Terraform apply...
info: {"awsSecurityGroup":null,"cloud":"gcp","driver":"github","id":"iterative-34qzkcov8n4ws","idleTimeout":1800,"image":null,"instanceGpu":null,"instanceHddSize":10,"instanceIp":"xxx","instanceLaunchTime":"2021-09-24 09:05:45.397746969 -0700 PDT m=+28.798050565","instanceType":"m","labels":"cml","name":"cml-2g7sdfdtiy","region":"us-west","repo":"xxx","single":true,"spot":false,"spotPrice":-1,"timeouts":null}

[DavidGOrtega]

Interesting that this is not working. It was as far as I remember. Maybe using undefined in the coerce function do the trick? If thats the case maybe is better than null explicitly which is going to be a difficult type to setup

[dacbd]

I tried changing the value and it seemed that if the result of the coerce function isn't in the choices array then it fails.
My very first patch was to set it to 'nogpu' instead of null which worked fine.
I opted to add null as an option instead to preserve the truthy/falsy behavior in ternary statements.

[dacbd]

I'll try this and we can see.

        choices: ['nogpu', 'k80', 'v100', 'tesla'],
        coerce: (val) => (val === 'nogpu' ? undefined : val),

[DavidGOrtega]

Thats what I suggested

[dacbd]

@DavidGOrtega It looks like undefined behaves as expected, Im going to do a full test in our gcp env and if it works as expected I'll update the PR.

[0x2b3bfa0]

See also yargs/yargs#756

[DavidGOrtega]

@DavidGOrtega It looks like undefined behaves as expected, Im going to do a full test in our gcp env and if it works as expected I'll update the PR.

@danielbarnes that was my intuition, since its allowing you to not set it, I would say that this worked also with null previously, but not sure... JS and its non "Good parts" 🙏

[dacbd]

JS and its non "Good parts" 🙏

🤣

[dacbd]

Updated to use undefined vs null works as expected from my test:

dabarnes@ubuntu:~/projects/cml$ ./bin/cml.js runner \
>     --single \
>     --log=debug \
>     --idle-timeout=1800 \
>     --token=xxx\
>     --cloud=gcp \
>     --cloud-region=us-west \
>     --cloud-type=m \
>     --cloud-gpu=nogpu \
>     --repo=xxx \
>     --driver=github \
>     --cloud-hdd-size=10
info: Preparing workdir /home/dabarnes/.cml/cml-jnnd5x6sq0...
info: Deploying cloud runner plan...
info: Terraform apply...
info: {"awsSecurityGroup":null,"cloud":"gcp","driver":"github","id":"iterative-2v4cyhm47w2q7","idleTimeout":1800,"image":null,"instanceGpu":null,"instanceHddSize":10,"instanceIp":"xxx","instanceLaunchTime":"2021-09-24 11:41:41.997215239 -0700 PDT m=+24.104635525","instanceType":"m","labels":"cml","name":"cml-jnnd5x6sq0","region":"us-west","repo":"xxx","single":true,"spot":false,"spotPrice":-1,"timeouts":null}

[0x2b3bfa0]

Thank you very much, @danielbarnes!

[DavidGOrtega]

Thank you very much, @danielbarnes!

I subscribe!!! Thanks @danielbarnes