Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

adds script for setting up dev env in ubuntu #393

Closed
wants to merge 4 commits into from

Conversation

Oishika-Pradhan
Copy link
Contributor

@Oishika-Pradhan Oishika-Pradhan commented May 18, 2018

Description

A bash script that sets up the dev env and hosts the application for (new) developers on ubuntu and osx.

Fixes #384

Type of Change:

  • Code

Code/Quality Assurance Only

  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

  • @Fenn-CS can you please check whether the script runs on ubuntu?
  • I have tested out the osx script

Checklist:

  • My PR follows the style guidelines of this project
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Code/Quality Assurance Only

  • My changes generate no new warnings

@coveralls
Copy link

coveralls commented May 18, 2018

Coverage Status

Coverage remained the same at 98.655% when pulling 6af8da9 on Oishika-Pradhan:develop into 2943851 on systers:develop.

@Fenn-CS Fenn-CS self-requested a review May 19, 2018 19:30
Fenn-CS
Fenn-CS previously approved these changes May 19, 2018
Copy link
Contributor

@Fenn-CS Fenn-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Fenn-CS
Copy link
Contributor

Fenn-CS commented May 19, 2018

@nida @prachi1210 ps review this

Copy link

@tonythomas01 tonythomas01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1

pip3 install virtualenv

# setting up virtualenv and installing requirements
virtualenv -p $(which python3) venv

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can lead to trouble. there are considerable differences between python 3.4 and python 3.5 etc. The only possible good solution seems to use https://github.com/pyenv/pyenv and point to a fixed python version. Like say python 3.6.5

@@ -0,0 +1,69 @@
#!/bin/bash

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename the command to say setup_env_linux_debian.sh or setup_env_linux_.sh. Ubuntu is a child derivative.

fi
done

for ppkg in "$PYTHON_PREREQ[@]"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just running a pip install -r requirements/dev.txt or something like that ?


if [ ${#MISSING[@]} -ne 0 ]; then
echo "Following required packages are missing, please install them first."
echo ${MISSING[*]}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally, your script should apt-get install all-those-packages-your-app-need.

export SECRET_KEY=foobarbaz
python ../systers_portal/manage.py migrate
python ../systers_portal/manage.py cities_light
python ../systers_portal.manage.py createsuperuser

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

../systers_portal/manage.py

@Fenn-CS
Copy link
Contributor

Fenn-CS commented May 20, 2018

@Oishika-Pradhan @tonythomas01 has made some change request.... They will make the script robust.

@Oishika-Pradhan
Copy link
Contributor Author

I'll make the necessary changes @Fenn-CS @tonythomas01

@Oishika-Pradhan
Copy link
Contributor Author

I have made the changes. Please look into it @tonythomas01 @Fenn-CS

sed -i.bak "s/'PASSWORD': '',/'PASSWORD': '$password',/" ../systers_portal/systers_portal/settings/dev.py

# host the server for the first time
export SECRET_KEY=foobarbaz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you export the key in typical string format? export SECRET_KEY='foobarbaz' What you did works anyways... @tonythomas01 can review this one more time and if its ok to him then we are good.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine as long as nobody is going to use this on production. 👍


virtualenv -p $(which python3.6) venv
source venv/bin/activate
pip install -r requirements/dev.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is inside the script directory this is meant to be run in the project root you want to change that to pip install -r ../requirements/dev.txt

apt-get install python3.6 python-pip
pip install virtualenv

virtualenv -p $(which python3.6) venv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should just create the virtual environment without specifying the python version virtaulenv will pick the latests or you can ask user to specif path to desired python version which should be 3.6. Because this construct leads to problems on ubuntu

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

virtualenv doesn't pick the latest version, it picks the default one. And the application is based on python3.6.
What we can do is ask for the desired path for the python3.6 binary to the user.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Oishika-Pradhan agreed on that. There is a problem tbh - apt-get install python3.6 might not work on some architectures (might even break some depending on how messed up their original install is). This is why I always recommend people to use a python version manager, like pyenv. Let me paste how this part should look like (copying from my own personal project)

Install pyenv using following scripts: https://github.com/pyenv/pyenv-installer. The installation requires you to add some things to your .bashrc or .zshrc file. In my case, it is:
$ curl -L https://github.com/pyenv/pyenv-installer/raw/master/bin/pyenv-installer | bash
...
export PATH="/home/user/.pyenv/bin:$PATH"
eval "$(pyenv init -)"
eval "$(pyenv virtualenv-init -)"
...
and source the rc file using source ~/.bashrc

on macos:
...
brew install pyenv
brew install pyenv-virtualenv
...
and add the following to your ~/.bash_profile (and after that do source ~/.bash_profile):

...
eval "$(pyenv init -)"
eval "$(pyenv virtualenv-init -)"
...

Now install the right python version using 
...
$ pyenv install 3.6.4 
...
Hint: If it shows something like: WARNING: The Python sqlite3 extension was not compiled. Missing the SQLite3 lib?, you can install it following something like: https://gist.github.com/antivanov/01ed4eac2d7486a170be598b5a0a4ac7. See that we are not using SQLite for local testing anymore, so you can actually ignore this.

create a virtual environment for the project: 
...
pyenv virtualenv 3.6.4 yourvenvname-p3
...
activate the virtual environment for the repository (this will create a .python-version file, which is ignored via .gitignore):
...
pyenv local yourvenvname-p3
...
- this would automatically activate the env as soon as you CD into the dir

install dependencies: 
...
pip install -r requirements-dev.txt
...


# create the database using psql and the user postgres
read -p "Enter username: " username
read -s -p "Enter password: " password
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a confirmation password is added and the two compared to prevent mistakes, because the password is not displayed.

Copy link

@tonythomas01 tonythomas01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 python3.6 will not work in <=16.04 ubuntu (and possibly other archs as well).

fi

# install system-wide dependencies
apt-get install python3.6 python-pip

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fails in Ubuntu 14.04 and 16.04

apt-get install python3.6 python-pip
pip install virtualenv

read -p "Python3.6 executable path: " pythonpath

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: is there a requirement for portal app to use a certain python 3.6 version ? If there is, I would just recommend using pyenv and save the day. I had pasted the instructions somewhere as I remember.

@Oishika-Pradhan
Copy link
Contributor Author

@tonythomas01 I have updated the scripts to use pyenv and pyenv-virtualenv. Please review.

tonythomas01
tonythomas01 previously approved these changes May 31, 2018
Copy link

@tonythomas01 tonythomas01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice looking very good but some minor nits, but I just approve :)

fi

# install system-wide dependencies
apt-get install python3 python-pip

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so when you are using pyenv, apt-get install python3 is not required. You can kill it. also, install curl and git as well.

@Oishika-Pradhan
Copy link
Contributor Author

Done @tonythomas01, thanks for reviewing.

@Fenn-CS
Copy link
Contributor

Fenn-CS commented Jun 1, 2018

@Oishika-Pradhan looks good at the level of executable commands and functionality however the script fails at user/db creation. The four fails match with all the four psql postgres -c " "
` commands

psql: FATAL:  role "root" does not exist
psql: FATAL:  role "root" does not exist
psql: FATAL:  role "root" does not exist
psql: FATAL:  role "root" does not exist
Traceback (most recent call last):
  File "../systers_portal/manage.py", line 8, in <module>
    from django.core.management import execute_from_command_line
ImportError: No module named django.core.management
Traceback (most recent call last):
  File "../systers_portal/manage.py", line 8, in <module>
    from django.core.management import execute_from_command_line
ImportError: No module named django.core.management
Traceback (most recent call last):
  File "../systers_portal/manage.py", line 8, in <module>
    from django.core.management import execute_from_command_line
ImportError: No module named django.core.management
Traceback (most recent call last):
  File "../systers_portal/manage.py", line 8, in <module>
    from django.core.management import execute_from_command_line
ImportError: No module named django.core.management

I believe in ubuntu we have to supply a password to use the postgres user, sure you have to include the -u flag or something please look at : https://stackoverflow.com/questions/16973018/createuser-could-not-connect-to-database-postgres-fatal-role-tom-does-not-e/16974197#16974197

@Oishika-Pradhan
Copy link
Contributor Author

@Fenn-CS I had to run psql from the user postgres. I have fixed it.

nida
nida previously approved these changes Jun 2, 2018
Copy link

@nida nida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.


# install system-wide dependencies
apt-get install git curl python-pip
curl -L https://github.com/pyenv/pyenv-installer/raw/master/bin/pyenv-installer | bash

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have to add postgresql installation scripts. also, probably

sudo apt-get install python-pip python-dev libpq-dev postgresql postgresql-contrib git curl

?

Copy link
Contributor

@Fenn-CS Fenn-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The my installation was successful. But the script at the level of running migrations because of SECRET_KEY... issues please adjust the PR according the comments. Also the script must be run inside the script folder others it would since the relative paths would break. But you should bear in mind that is possible for someone to run the script out of the scripts folder...That means after your final changes you need to update the documentation (setup guide)

sed -i.bak "s/'NAME': 'systersdb',/'NAME': '$db',/" ../systers_portal/systers_portal/settings/dev.py
sed -i.bak "s/'USER': '',/'USER': '$username',/" ../systers_portal/systers_portal/settings/dev.py
sed -i.bak "s/'PASSWORD': '',/'PASSWORD': '$password',/" ../systers_portal/systers_portal/settings/dev.py

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the secret key is ideally export immediately after the virtual environment is created.


elif [ $1 == 'run' ]; then
# host the development server
export SECRET_KEY='foobarbaz'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move export SECRET_KEY=foobar to the before all python ... instructions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fenn-CS when a developer would run ./devenv-debian run just to host the server, the env variable SECRET_KEY must be initialized, and the other python ... commands don't need it anyways.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But setup is failing on ubuntu... Complaining of SECRET_KEY not found in path... Anything that would access the manage.py file will need that e.g python ../systers_portal/manage.py migrate you can't run that without setting the SECRET_KEY first....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added export SECRET_KEY='foobarbaz' before all the python commads, please check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.

- argument 'run' hosts the server
- argument 'setup' sets up the devenv
- changed the names of the scripts for easier usability
- added argument error handling
@Fenn-CS
Copy link
Contributor

Fenn-CS commented Jun 13, 2018

I just hope the OSX script has been tested properly... As I have concentrated on the debian version which Looks good.

@tonythomas01
Copy link

tonythomas01 commented Jun 13, 2018

This has matured enough. Anything that shows up later can be caught and fixed later I guess. Also, with python and bash - its not easy to write a 100% always working solution.

@rpattath rpattath added Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Jun 1, 2020
@theyashshahs
Copy link
Contributor

@Oishika-Pradhan resolve the merge conflicts :)

Copy link
Contributor

@theyashshahs theyashshahs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the merge conflicts

@theyashshahs
Copy link
Contributor

theyashshahs commented Jul 4, 2020

@Oishika-Pradhan change the PR title to follow the commit guidelines and resolve the merge conflicts :)

@theyashshahs
Copy link
Contributor

ping @Oishika-Pradhan

@SanketDG
Copy link
Contributor

@anitab-org/portal-maintainers @anitab-org/portal-managers Do we really need this?

IMO having manual instructions + Docker is enough right now, also we are playing with the idea of pre-made environments.

@sakshi1499
Copy link

@SanketDG I totally agree! Yes in the last session we did discuss about CodeSandbox and other options. Closing this PR.

@sakshi1499 sakshi1499 closed this Aug 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write automation scripts for dev-enviroment set up
9 participants