cli: add demo functionality to Reana cli script #19

hjhsalo · 2017-04-27T16:24:46Z

add cli command to run reana-demo-helloworld example easily
reana run helloworld

Signed-off-by: Harri Hirvonsalo harri.hirvonsalo@cern.ch

hjhsalo · 2017-04-27T16:32:03Z

For now, hardcoded to run only helloworld-workflow with 'atlas' as experiment.

Doesn't follow the run-syntax described in reanahub/reana-demo-helloworld#2 as I feel that this is something we will have to discuss more before implementing it; what is the actual input of reana run (file? folder?) and how to submit the workflow to workflow-controller, etc.

tiborsimko · 2017-04-27T16:37:49Z

scripts/reana

@@ -135,6 +222,7 @@ usage () {
    echo "  install-minikube    Installs and starts local minikube instance"
    echo "  init                Initialize REANA system"
    echo "  down                Delete REANA components"
+    echo "  demo helloworld     Runs a helloworld demo"


run helloworld so that we are ready for the future run myanalysis without having to change the CLI.

tiborsimko · 2017-04-27T16:38:39Z

scripts/reana

+                    #kubectl logs -f $(kubectl get pods | grep -Po 'yadage-atlas-worker[^ ]+')
+
+                    # Open workflow-monitor web-interface
+                    firefox "http://$controller_url/$workflow_id"


It would be good to take commands like firefox out of the script (and use them only in docs).

Changed to open and xdg-open.

tiborsimko · 2017-04-27T16:39:28Z

.gitignore

+# IntelliJ project files
+.idea
+*.iml
+out


out needed here?

Have to check if this is copy-paste error.

Legit but not needed. I'll remove it.
IntelliJ -specific folder for generated .class files and artifacts

tiborsimko · 2017-04-27T16:41:10Z

scripts/reana

+    # The code to check for flags is from http://stackoverflow.com/q/2875424
+    # Not sure if this is a good way or not.
+    flag_verbose=$(echo "$*" | grep -e "--verbose" -o)
+    flag_devel=$(echo "$*" | grep -e "--devel" -o)


Maybe simple quotes to prevent command execution:

$ echo "$(date)" Thu 27 Apr 18:40:02 CEST 2017 $ echo '$(date)' $(date)

or rely on the flag being present last in $3 some place, or use environment variable...

If I change this to flag_devel=$(echo $* | grep -e "--devel" -o), shellcheck complains:

SC2048: Use "$@" (with quotes) to prevent whitespace problems.

SC2086: Double quote to prevent globbing and word splitting.

With flag_devel=$(echo $@ | grep -e "--devel" -o) shellcheck complains:

SC2068: Double quote array expansions, otherwise they're like $* and break on spaces.

If I use flag_devel=$(echo "$@" | grep -e "--devel" -o) shellcheck doesn't complain.

I understand your point though.

I don't know enough about shell scripting to be able to say if any of these is good.

Should we change from sh to bash to use if [[ "$@" != *"--devel"* ]]; then if that would be a better alternative?

tiborsimko · 2017-04-27T16:43:18Z

scripts/reana

+                            -H "Content-Type: application/json" \
+                            -X POST -d \
+                            "{\"experiment\":\"atlas\",
+                              \"toplevel\":\"github:diegodelemos/reana-demo-helloworld@2-add-helloworld-workflow\",


diegodelemos/... image location not needed anymore? Use central one?

@diegodelemos ?

see reanahub/reana-demo-helloworld#4. Once it is merged \"toplevel\":\"github:diegodelemos/reana-demo-helloworld@2-add-helloworld-workflow\" should be changed for \"toplevel\":\"github:reanahub/reana-demo-helloworld\".

diegodelemos · 2017-04-28T07:06:47Z

scripts/reana

+
+                if [ "$flag_verbose" != "" ]; then
+                    echo "[INFO] Open http://$controller_url/$workflow_id to see visual representation of your workflow and it's progress."
+                    echo "[INFO] You can 'cat' or 'tail' /reana/atlas/$workflow_id/yadage/helloworld/greetings.txt -file"


Because of reanahub/reana-workflow-engine-yadage#20 only ATLAS workflows are currently working, however, it would be nice to use something line $(experiment) instead of the string atlas. Since in reana-resources-X we are following the pattern /reana/<experiment> (see here) it would match perfectly

diegodelemos · 2017-04-28T07:08:43Z

scripts/reana

+                    echo "[INFO] Open http://$controller_url/$workflow_id to see visual representation of your workflow and it's progress."
+                    echo "[INFO] You can 'cat' or 'tail' /reana/atlas/$workflow_id/yadage/helloworld/greetings.txt -file"
+                    echo "       from shared storage of REANA cluster to see how helloworld workflow progresses."
+                    echo "[INFO] (e.g. minikube ssh \"tail -c+1 -F /reana/atlas/$workflow_id/yadage/helloworld/greetings.txt\")"


$(experiment) rather than atlas

diegodelemos · 2017-04-28T07:09:10Z

scripts/reana

+                echo ""
+                echo "Workflow_id: $workflow_id"
+                echo "Monitor: http://$controller_url/$workflow_id"
+                echo "Output:  /reana/atlas/$workflow_id/yadage/helloworld/greetings.txt"


same about atlas

diegodelemos · 2017-04-28T07:10:49Z

scripts/reana

+                    # Tail output-file of the workflow
+                    echo "[INFO] Tailing...hit Ctrl+C when done."
+                    echo ""
+                    minikube ssh "tail -c+1 -F /reana/atlas/$workflow_id/yadage/helloworld/greetings.txt"


same about atlas

diegodelemos · 2017-04-28T08:28:35Z

scripts/reana

+                              \"preset_pars\": {\"name\": \"J.Doe\", \"delay\": \"2\"}
+                            }" \
+                            http://"$(reana get reana-workflow-controller)"/yadage \
+                            | grep -oP '(?<="workflow_id": ")[^"]*')


-P option is not available in BSD, so something like grep -oE '"workflow_id"\:[[:space:]]*\"[^\"]*\"'| cut -d ":" -f 2 | tr -d "\"" would support both GNU and BSD

Didn't update yet. I will look into this next week.

This is not yet update, it prevents successful execution on Mac

diegodelemos · 2017-04-28T08:39:29Z

scripts/reana

+
+                if [ "$flag_devel" != "" ]; then
+                    # Start tailing of job-controller logs
+                    #kubectl logs -f $(kubectl get pods | grep -Po 'job-controller[^ ]+')


I see more practical to have some developer documentation where we explain which logs would be nice to follow as well as which endpoints should be opened. If you use i3, tmux or something else you have your own setup for doing this kind of things

hjhsalo · 2017-04-28T16:55:24Z

scripts/reana

+        echo "Output:  /reana/$experiment/$workflow_id/yadage/helloworld/greetings.txt"
+        echo ""
+
+        if [ "$flag_devel" != "" ]; then


Should I get rid of the whole --devel part of the code?

We could leave it there if we find it useful for development, all in all it is an option..

hjhsalo · 2017-04-28T16:56:45Z

scripts/reana

+
+    # The code to check for flags is from http://stackoverflow.com/q/2875424
+    # Not sure if this is a good way (POSIX compliant?) or not.
+    flag_verbose=$(echo "$@" | grep -e "--verbose" -o)


If I change this to flag_devel=$(echo $* | grep -e "--devel" -o), shellcheck complains:

SC2048: Use "$@" (with quotes) to prevent whitespace problems.

SC2086: Double quote to prevent globbing and word splitting.

With flag_devel=$(echo $@ | grep -e "--devel" -o) shellcheck complains:

SC2068: Double quote array expansions, otherwise they're like $* and break on spaces.

If I use flag_devel=$(echo "$@" | grep -e "--devel" -o) shellcheck doesn't complain.

I understand your point though.

I don't know enough about shell scripting to be able to say if any of these is good.

Should we change from sh to bash to use if [[ "$@" != *"--devel"* ]]; then if that would be a better alternative?

flag_devel=$(echo "$@" | grep -qe "--\<verbose\>")
flag_devel=$(echo "$@" | grep -qe "--\<devel\>")
might be better because current one would allow providing --dev or --verbo. So we could stick to sh which is more standard

I think I got your point. Have to admit that I'm not sure if I'm actually restricting matches with -o. It should show only the part which matches pattern.
Tested based on suggestion and I couldn't get grep -qe "--\<verbose\>" to work.
How about flag_verbose=$(echo "$@" | grep -oe "--\<verbose\>")?

hjhsalo · 2017-04-28T16:59:59Z

scripts/reana

@@ -26,6 +26,8 @@ reana-workflow-monitor
 reana-message-broker
 reana-workflow-engine-yadage"

+experiment_list=$(find ./configuration-manifests/namespaces/ -name \*.yaml -printf '%f\n' |  sed -e 's/\.yaml$//')


Should I just use hardcoded experiment list?

It should be ok, in the future we can grab it from configuration

which is what you actually do, sorry about that 🙃

-printf is not supported on BSD, something like:
find ./configuration-manifests/namespaces/ -name \*.yaml -type f -exec basename {} .yaml ";"
should be more portable

diegodelemos · 2017-05-03T09:37:35Z

scripts/reana

+    if [ "$3" = "-e" ]; then
+        experiment=$4
+    else
+        experiment=${REANA_EXPERIMENT-}


Is the trailing - necessary? In -h it doesn't include it

Runs a helloworld demo in namespace of specified experiment (e.g. '-e atlas'). Specify experiment with '-e' flag or $REANA_EXPERIMENT environment variable.'

Quote from some SO-post:
"${var:-} and ${var-} mean different things. ${var:-} will expand to empty string if var unset or null and ${var-} will expand to empty string only if unset"
BUT now that I think of it, it might be better with ${REANA_EXPERIMENT:-} so it will default to empty string.

Shouldn't we warn the user to either set -e <experiment> or the REANA_EXPERIMENT env var? otherwise the request could contain an empty string

* add cli command to run reana-demo-helloworld example easily `reana run helloworld` * prepare for run syntax ('reana run myworkflow') described in reanahub/reana-demo-helloworld/issues/2 Signed-off-by: Harri Hirvonsalo <harri.hirvonsalo@cern.ch>

diegodelemos

LGTM

hjhsalo added the Status: in review label Apr 27, 2017

tiborsimko reviewed Apr 27, 2017

View reviewed changes

diegodelemos mentioned this pull request Apr 28, 2017

workflow: add yadage workflow file reanahub/reana-demo-helloworld#4

Merged

diegodelemos reviewed Apr 28, 2017

View reviewed changes

hjhsalo force-pushed the demo-option-for-cli branch 2 times, most recently from ee2b97c to 5c8b4ab Compare April 28, 2017 16:41

hjhsalo commented Apr 28, 2017

View reviewed changes

diegodelemos reviewed May 3, 2017

View reviewed changes

hjhsalo force-pushed the demo-option-for-cli branch 3 times, most recently from 7ba0d4f to d7f22a1 Compare May 3, 2017 12:46

hjhsalo force-pushed the demo-option-for-cli branch from d7f22a1 to b37ccfe Compare May 3, 2017 14:28

diegodelemos approved these changes May 4, 2017

View reviewed changes

diegodelemos merged commit b37ccfe into reanahub:master May 4, 2017

diegodelemos removed the Status: in review label May 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add demo functionality to Reana cli script #19

cli: add demo functionality to Reana cli script #19

hjhsalo commented Apr 27, 2017 •

edited

Loading

hjhsalo commented Apr 27, 2017 •

edited

Loading

tiborsimko Apr 27, 2017

tiborsimko Apr 27, 2017

hjhsalo Apr 28, 2017

tiborsimko Apr 27, 2017

hjhsalo Apr 27, 2017

hjhsalo Apr 27, 2017

tiborsimko Apr 27, 2017

hjhsalo Apr 28, 2017 •

edited

Loading

tiborsimko Apr 27, 2017 •

edited

Loading

hjhsalo Apr 27, 2017

diegodelemos Apr 28, 2017

diegodelemos Apr 28, 2017

diegodelemos Apr 28, 2017

diegodelemos Apr 28, 2017

diegodelemos Apr 28, 2017

diegodelemos Apr 28, 2017

hjhsalo Apr 28, 2017

diegodelemos May 3, 2017

diegodelemos Apr 28, 2017

hjhsalo Apr 28, 2017

diegodelemos May 3, 2017

hjhsalo Apr 28, 2017

diegodelemos May 3, 2017

hjhsalo May 3, 2017

hjhsalo Apr 28, 2017

diegodelemos May 3, 2017

diegodelemos May 3, 2017

diegodelemos May 3, 2017

diegodelemos May 3, 2017

hjhsalo May 3, 2017

diegodelemos May 3, 2017

diegodelemos left a comment

cli: add demo functionality to Reana cli script #19

cli: add demo functionality to Reana cli script #19

Conversation

hjhsalo commented Apr 27, 2017 • edited Loading

hjhsalo commented Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjhsalo Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

tiborsimko Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegodelemos left a comment

Choose a reason for hiding this comment

hjhsalo commented Apr 27, 2017 •

edited

Loading

hjhsalo commented Apr 27, 2017 •

edited

Loading

hjhsalo Apr 28, 2017 •

edited

Loading

tiborsimko Apr 27, 2017 •

edited

Loading