non interactive ww3_from_ftp.sh #603
Conversation
add option --no-check-certificate
mickaelaccensi
added
the
bin directory change
aliabdolali
left a comment
aliabdolali
left a comment
There was a problem hiding this comment.
I tested that at my end, it worked as expected.
JessicaMeixner-NOAA
left a comment
JessicaMeixner-NOAA
left a comment
There was a problem hiding this comment.
I'm not opposed to this PR as is, but had some different thoughts when I originally made the issue #578 so thought I'd get your thoughts on that before we merge.
| if [ $# -eq 3 ] ; then | ||
| interactive='n' | ||
| ww3dir=$1 # [ '../../'] | ||
| wnew=$2 # ['y'] |
There was a problem hiding this comment.
Should we combine this to just have one option?
| interactive='y' | ||
| if [ $# -eq 3 ] ; then | ||
| interactive='n' | ||
| ww3dir=$1 # [ '../../'] |
There was a problem hiding this comment.
To be more consistent with other scripts should we update this to be pointing to the model directory instead of the top level directory?
| echo -e "be '../../' if in the model/bin directory or '.' if already in the " | ||
| echo -e "top/main directory:" | ||
| read ww3dir | ||
| if [ "$interactive" = "n" ] |
There was a problem hiding this comment.
This is nice as it allows for previous functionality//backwards compatibility, but we're at a point where we're about to have some big changes and therefore are we willing to break this and just make this a script where you have to give it the model directory path and then you either keep things (optional, make an option) or the default is you do nothing and it doesn't keep things?
|
let me know if this version fits better to your expectations : |
|
Okay, I did something wrong: If we have -k for keeping the tar files, do we still need to have the "y y"? I thought that was the default behavior now. |
| echo -e "\n\n This script will download data from the ftp for WAVEWATCH III " | ||
| echo -e "Enter the relative path to the main/top level directory, this would " | ||
| echo -e "be '../../' if in the model/bin directory or '.' if already in the " | ||
| echo -e "top/main directory:" |
There was a problem hiding this comment.
If we're giving the path to the model directory, we should change this description or is this still the same?
you don't need anymore to specify the model directory since it is detected automatically |
|
I've added usage to make it clearer and I'm also updating the PR summary |
mickaelaccensi
changed the title
correct option to keep tar files
|
ok this version should be the final one unless you see something to change/improve |
|
This is AWESOME!!! Thank you so much @mickaelaccensi |
JessicaMeixner-NOAA
left a comment
JessicaMeixner-NOAA
left a comment
There was a problem hiding this comment.
The new usage and -h options are really nice. Great improvement! Thank you for implementing this update.
* Update utest script: Handle global, regional, and coupled tests * Update CI workflow: Instead of saving the docker image (~730 MB) as an artifact, only save the executable (~77 MB) * Add regional thread and decomposition tests to CI * Add coupled restart test to CI * Fix to run threads on its own core in ec2 for thread tests * Use a slightly different ubuntu-hpc image to match the uid and gid within ec2 instances * Exclude L127 control test temporarily
3 participants
Pull Request Summary
change default behavior to automatically download tar files and detect main directory
add options :
-h : print usage
-i : interactive mode
-k : keep tar files
add download option --no-check-certificate
Description
solve issue #578
the new way to use it is to add arguments in the command line :
cd WW3/model/bin/ww3_from_ftp.shIssue(s) addressed
fixes #578
Commit Message
non interactive ww3_from_ftp.sh
Check list
Testing
by running the bash script itself