5
\$\begingroup\$

I have a few Flask projects that I want to install, each in their own virtual environment. The approach is derived from this tutorial. The steps to create the VE's are very similar, so they're easy to automate. So I wrote this, as a first step. It seems to work OK:

#! /bin/bash 
if [ $# -lt 2 ] 
 then 
 echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name" 
 exit 
fi 
echo project name: 1ドル 
echo project user: 2ドル 
# create the webapp user and its home directory, and give it ownership 
sudo useradd --system --gid webapps --shell /bin/bash --home /mnt/u1/1ドル 2ドル 
sudo mkdir -p /mnt/u1/1ドル 
sudo chown 2ドル /mnt/u1/1ドル 
# become the webapp user, create a virtual environment, start it, and install gunicorn and flask 
sudo su 2ドル -c "cd ~; virtualenv ./venv" 
sudo su 2ドル -c "cd ~;. venv/bin/activate; pip install gunicorn flask"
echo -e "Done! Start with:\n$ sudo su - 2ドル\n$ . venv/bin/activate" 
echo -e "Or undo the whole kaboodle with:\n$ sudo userdel -r 2ドル" 
# at this point, manual configuration is necessary, which is different for each project. 

I'm fairly new to bash programming, so I'm open to suggestions.
Is this good practice at all?
Any serious pitfalls I have overlooked?
Any room for improvement?

asked Jan 3, 2017 at 20:47
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

There are a number of good practice items you are missing here, that would benefit your script.

First up, you should name your variables/parameters. The check you have to ensure that the project name and user are specified is OK, but you should rather pull the 1ドル and 2ドル values in to their own variables, say pname and puser.

pname=1ドル
puser=2ドル

Then it becomes easier to read the code... $pname is a lot easier to understand than 1ドル.

The second issue I see is that you don't check for error conditions. A relatively easy handler is to add the exit-on-error flag to your shell script. Consider adding set -e at the beginning of the script (or a more elaborate system, but your script should be fine with set -e. For more information see: What does set -e mean in a bash script?

The third issue is a small one - why are you using both sudo and su in the same command..... sudo su .... seems rhetorical. Just use sudo --user $puser ....

Another little issue is that you should exit with a non-zero if the parameters are wrong (use exit 1 instead of exit)...

echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name" 
exit

should be:

echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name" 
exit 1 

Yet another small issue is that it's normal to not have .sh as the extension for exectuable shell scripts. Why call your script mkve.sh instead of mkve? Also, you call it two different things in the error message, mkve.sh and mkve.bash.... which is right?

Finally, it is unconventional to put home directories in the /mnt folder. That folder has a special purpose when mounting file systems that are not normally mounted (things like CD's, USB drives, etc.). Using that folder for these home directories will probably lead to unexpected issues if/when someone mounts a drove over /mnt and all your home folders disappear.

The end code would be something like (untested):

#! /bin/bash 
# exit if there's a failed command
set -e
if [ $# -lt 2 ] 
 then 
 echo -e "mkve - make virtual environment\nUsage: mkve project_name user_name" 
 exit 2
fi
pname=1ドル
puser=2ドル
echo project name: $pname
echo project user: $puser
# create the webapp user and its home directory, and give it ownership 
sudo useradd --system --gid webapps --shell /bin/bash --home /ves/$pname $puser 
sudo mkdir -p /mnt/u1/$pname
sudo chown $puser /mnt/u1/$pname
# become the webapp user, create a virtual environment, start it, and install gunicorn and flask 
sudo --user $puser bash -c "cd ~; virtualenv ./venv" 
sudo --user $puser bash -c "cd ~;. venv/bin/activate; pip install gunicorn flask"
echo -e "Done! Start with:\n$ sudo -u $puser -i\n$ . venv/bin/activate" 
echo -e "Or undo the whole kaboodle with:\n$ sudo userdel -r $puser" 
# at this point, manual configuration is necessary, which is different for each project. 
answered Jan 3, 2017 at 22:32
\$\endgroup\$
2
  • \$\begingroup\$ Many thanks! I've copied most of your changes. Any reason for exit 2, rather than exit 1? To answer your questions, I'm using .sh because I get syntax highlighting in my editor. Second, I'm using /mnt/u1 because I'm putting those apps on a USB-stick in a Raspberry Pi. Its system disk is an SD-card and I want to avoid frequent writes to it, see here. Had an SD-card bricked mid-project twice, so far. Very inconvenient. A USB stick is a bit easier to backup and replace (althought I admit that's debatable.) \$\endgroup\$ Commented Jan 4, 2017 at 19:54
  • \$\begingroup\$ echo -e is also an antipattern, maybe switch to printf instead. \$\endgroup\$ Commented Feb 9, 2017 at 13:09

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.