Update env.sh to work on Mac OSX #78
Conversation
TESTING Done on macOS Mojave 10.14.6: ``` $ cd deepracer/rl_coach/ $ env | grep S3 $ env | grep LOCAL $ source env.sh $ env | grep S3 MODEL_S3_BUCKET=bucket MODEL_S3_PREFIX=rl-deepracer-sagemaker S3_ENDPOINT_URL=http://192.168.1.207:9000 $ env | grep LOCAL LOCAL=True LOCAL_ENV_VAR_JSON_PATH=/Users/dj/Code/deepracer/rl_coach/env_vars.json ```
|
For me it's also working under the same version. Some Linux user should test it and accept that PR. |
rl_coach/env.sh
Outdated
| export LOCAL_ENV_VAR_JSON_PATH=$(readlink -f ./env_vars.json) | ||
|
|
||
| # Check if the "readlink -f" command works | ||
| if readlink -f ./env_vars.json 2>/dev/null ; |
There was a problem hiding this comment.
Could probably change this to if [[ -X $(which readlink) ]] ; to test if the file really exists, and is executable, rather than invoking it and assuming the output will be testable. You could alse change the else to an else if and check the existence of greadlink as well
There was a problem hiding this comment.
We do need to check readlink -f because Mac 10.14.6 has readlink, it just doesn't support the -f option:
$ which readlink
/usr/bin/readlink
$ readlink -f
readlink: illegal option -- f
usage: readlink [-n] [file ...]
But I added the test you suggested, checked for greadlink too, and added error handling.
Also ensure that stdout is redirected along with stderr so the location doesn't get printed to the console. Add error case handling too.
|
Images have been progressed and https://github.com/aws-deepracer-community/deepracer-for-cloud works on Macs as well. I will close this PR |
Fix for #11.
TESTING
Done on macOS Mojave 10.14.6: