patternbashMinor
Bash script prepares virtual environment
Viewed 0 times
scriptenvironmentpreparesvirtualbash
Problem
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:
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?
#! /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?
Solution
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
Then it becomes easier to read the code...
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
The third issue is a small one - why are you using both
Another little issue is that you should exit with a non-zero if the parameters are wrong (use
should be:
Yet another small issue is that it's normal to not have
Finally, it is unconventional to put home directories in the
The end code would be something like (untested):
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=$2Then 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"
exitshould be:
echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"
exit 1Yet 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.Code Snippets
pname=$1
puser=$2echo -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"
exitecho -e "mkve.sh - make virtual environment\nUsage: mkve.bash project_name user_name"
exit 1#! /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.Context
StackExchange Code Review Q#151600, answer score: 2
Revisions (0)
No revisions yet.