Added support for PostGIS #9
Conversation
This change prevents out-of-memory errors by taking gunicorn out of the equation. Each part of the script starts a fresh container that only runs the script command, instead of running everything alongside a container that also runs gunicorn. Additionally, `make test` assumes it is running in an environment with the appropriate environment variables configured.
Alpine has a slightly steeper learning curve than Jessie. This switch makes the project easier for others to work with and maintain.
|
Feel free to "Rebase and merge" when ready. |
| - docker exec -t woeip.app make quality | ||
| - docker exec -t woeip.app make static | ||
| - docker exec -t woeip.app make test | ||
| - docker-compose -f docker-compose.yml -f docker-compose.travis.yml run app "make quality" |
There was a problem hiding this comment.
Interesting way to solve the problem: create a compose configuration specifically for travis. Noting the difference between production and travis compose will help teach me a little more about both- and exactly how gunicorn does its thing.
| python3-dev \ | ||
| && pip install pipenv \ | ||
| && rm -rf /var/cache/apk/* | ||
| RUN apt-get update && \ |
There was a problem hiding this comment.
The bash commands are definitely welcome sight. I'm more familiar with debian/ubuntu. An unintended consequence of switching from alpine is that its making it easier for me to decode how this all works.
| @@ -36,7 +36,7 @@ production-requirements: ## Install requirements for production | |||
| pipenv install | |||
|
|
|||
| test: clean ## Run tests and generate coverage report | |||
| SECRET_KEY=fake DATABASE_URL="sqlite://:memory:" coverage run -m pytest --durations=25 -v | |||
| coverage run -m pytest --durations=25 -v | |||
There was a problem hiding this comment.
From a quick glance, this appears to mean that the test database will switch from sqlite (which would've needed to be spatialite to support gis capabilities) to postgis
There was a problem hiding this comment.
Correct. The tests now use whatever database is configured via DATABASE_URL. This is ideal because you avoid potentially having tests that work for one database backend but not another. As the project grows, you may wish to go back to using SQLite for speed. Just be aware of the tradeoff.
| environment: | ||
| - DEBUG=true | ||
| - SECRET_KEY=replace-me | ||
| - DATABASE_URL=psql://postgres:postgres@woeip.db:5432/woeip?connect_timeout=60 | ||
| - DATABASE_URL=postgis://postgres:postgres@woeip.db:5432/woeip?connect_timeout=60 |
There was a problem hiding this comment.
Obligatory refractoring, to upgrade from gres to gis
| @@ -26,6 +26,7 @@ | |||
| 'django.contrib.messages', | |||
| 'django.contrib.staticfiles', | |||
| 'django.contrib.flatpages', | |||
| 'django.contrib.gis', | |||
There was a problem hiding this comment.
Looks like we're geodjango enabled too
|
I'd like to give @r-b-g-b and @YotamHa @Ethan-bradley a chance to review it while it's still a pull request. I think pull requests are a good pacing mark, to help everyone learn the code base together. After all, this is a lot for us to learn. |
|
I'm merging now. You can continue posting comments and questions to PRs after merging, so keep them coming. I'll answer as time allows. |
This is a redux of #5 and #6. The primary difference is the first commit, which updates the Travis build.
The Travis build was previously failing, with processes exiting with code 137. This exit code usually indicates the process was killed because the system ran out of memory. Since the processes were running in the
appcontainer, the only other process that would kill the memory isgunicorn.That process is no longer run on Travis. Instead we restart the containers for every step of the build script, each time running only the process necessary to complete that part of the script.