5 ms·

I wouldn't trust that script too much: there is no error checking at all. What happens when s3cmd fails and after two months you discover the "Vault sync succe

I wouldn't trust that script too much: there is no error checking at all. What happens when s3cmd fails and after two months you discover the "Vault sync succeded" emails you have been getting were all illusory? If the author is reading: please take the time to update your example, including proper error checking (at least "set -eu"), otherwise the people on the internet that are going to copy your script are in for a hard time. Skeleton of the "incriminated" script: #!/bin/bash [...] s3cmd sync ... [...] notify-send [...] "Vault sync succeded [...]"

mitjafelicijan 6y ago
I agree with you. I should add a disclaimer that this is still being worked on. I will add what you proposed. Thanks for your input.
- lanstin 6y ago
  Always start all your bash scripts with set -e You'll never regret it and might be very very very happy it was there. And if you start with that, then as you work on the shell script, you'll be more likely to make the script be idempotent (checking for files before copying, checking for lines in files before appending/sed'ing etc.) Idempotent >> non-idempotent for "alter the state on an end point" things.
  - mitjafelicijan 6y ago
    Will do. Thanks for this.
  - slaymaker1907 6y ago
    Totally agreed. You can also temporarily turn it off and turn it back on again if you really need to by doing "set +e ... set -e". Sometimes it is more convenient to check errors explicitly, i.e. with test code where the test is expected to fail, but errors should fail fast by default.
  - jbrot 6y ago
    I don’t know about never regretting. “set -e” is the right choice the vast majority of the time, but definitely has some gotchas to it. For instance, what does the following script print? #!/bin/bash set -e ( echo "Start of Subshell" (exit 1) echo "This shouldn’t run, right?" ) || echo "Subshell failed"
  - deleted 6y ago
    [deleted]
  - jamiesonbecker 6y ago
    Also, "unofficial bash strict mode" is terrific, especially if you intend to distribute a script: http://redsymbol.net/articles/unofficial-bash-strict-mode/ http://redsymbol.net/articles/unofficial-bash-strict-mode/ set -euo pipefail IFS=$'\n\t' (You can always turn, for example, error checking back off for an individual portion of the script with `set +e`)
    - m3adow 6y ago
      Adding "-E" should be considered as well, so traps are also applied in functions.
    - jonahx 6y ago
      The options are good, but I don't like the IFS suggestion. Bash has a perfectly good way of dealing with spaces in array elements. Fixing the example in the article is as simple as adding double quotes around the array: for name in "${names[@]}"; do echo "$name" done The will not result in a single element, but will destructure each element of names, as if each were double-quoted, and will print the same output that the "strict mode IFS setting" does. What's more, the double-quoted array solution will work even if the elements contain newlines or tabs.
gtirloni 6y ago
I haven't looked at the code but the author would benefit from using shellcheck. https://www.shellcheck.net https://www.shellcheck.net
notyourday 6y ago
Whyyyyyy? Just use rclone.

mitjafelicijan 6y ago

I agree with you. I should add a disclaimer that this is still being worked on. I will add what you proposed. Thanks for your input.

lanstin 6y ago
Always start all your bash scripts with set -e You'll never regret it and might be very very very happy it was there. And if you start with that, then as you work on the shell script, you'll be more likely to make the script be idempotent (checking for files before copying, checking for lines in files before appending/sed'ing etc.) Idempotent >> non-idempotent for "alter the state on an end point" things.
- mitjafelicijan 6y ago
  Will do. Thanks for this.
- slaymaker1907 6y ago
  Totally agreed. You can also temporarily turn it off and turn it back on again if you really need to by doing "set +e ... set -e". Sometimes it is more convenient to check errors explicitly, i.e. with test code where the test is expected to fail, but errors should fail fast by default.
- jbrot 6y ago
  I don’t know about never regretting. “set -e” is the right choice the vast majority of the time, but definitely has some gotchas to it. For instance, what does the following script print? #!/bin/bash set -e ( echo "Start of Subshell" (exit 1) echo "This shouldn’t run, right?" ) || echo "Subshell failed"
- deleted 6y ago
  [deleted]
- jamiesonbecker 6y ago
  Also, "unofficial bash strict mode" is terrific, especially if you intend to distribute a script: http://redsymbol.net/articles/unofficial-bash-strict-mode/ http://redsymbol.net/articles/unofficial-bash-strict-mode/ set -euo pipefail IFS=$'\n\t' (You can always turn, for example, error checking back off for an individual portion of the script with `set +e`)
  - m3adow 6y ago
    Adding "-E" should be considered as well, so traps are also applied in functions.
  - jonahx 6y ago
    The options are good, but I don't like the IFS suggestion. Bash has a perfectly good way of dealing with spaces in array elements. Fixing the example in the article is as simple as adding double quotes around the array: for name in "${names[@]}"; do echo "$name" done The will not result in a single element, but will destructure each element of names, as if each were double-quoted, and will print the same output that the "strict mode IFS setting" does. What's more, the double-quoted array solution will work even if the elements contain newlines or tabs.

gtirloni 6y ago

I haven't looked at the code but the author would benefit from using shellcheck. https://www.shellcheck.net https://www.shellcheck.net

notyourday 6y ago

Whyyyyyy? Just use rclone.