From 4b7ed9db2f0dcdc70bc3349beb8b4ba089086d26 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 28 Dec 2017 12:03:04 +0000 Subject: [PATCH 1/7] upgrade celery to 4.2.0 celery 4.2.0 no longer requires boto2, so we can get rid of that patched version. --- requirements-app.txt | 4 +--- requirements.txt | 13 +++++-------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/requirements-app.txt b/requirements-app.txt index a92646288..dc9f43f31 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -2,7 +2,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery==3.1.26.post2 # pyup: <4 +celery==4.2.1 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 @@ -28,5 +28,3 @@ awscli-cwlogs>=1.4,<1.5 botocore<1.11.0 git+https://github.com/alphagov/notifications-utils.git@30.4.0#egg=notifications-utils==30.4.0 - -git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/requirements.txt b/requirements.txt index b320175ed..2a2622a60 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery==3.1.26.post2 # pyup: <4 +celery==4.2.1 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 @@ -31,14 +31,11 @@ botocore<1.11.0 git+https://github.com/alphagov/notifications-utils.git@30.4.0#egg=notifications-utils==30.4.0 -git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 - ## The following requirements were added by pip freeze: alembic==1.0.0 -amqp==1.4.9 -anyjson==0.3.3 +amqp==2.3.2 bcrypt==3.1.4 -billiard==3.3.0.23 +billiard==3.5.0.4 bleach==2.1.3 boto3==1.6.16 certifi==2018.8.24 @@ -54,7 +51,7 @@ idna==2.7 itsdangerous==0.24 Jinja2==2.10 jmespath==0.9.3 -kombu==3.0.37 +kombu==4.2.1 Mako==1.0.7 MarkupSafe==1.0 mistune==0.8.3 @@ -77,6 +74,6 @@ six==1.11.0 smartypants==2.0.1 statsd==3.2.2 urllib3==1.23 +vine==1.1.4 webencodings==0.5.1 Werkzeug==0.14.1 -wheel==0.24.0 From 1786f99400c8b80ed6d37f4a7b79e9f5e2929a4f Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 28 Dec 2017 12:04:19 +0000 Subject: [PATCH 2/7] construct celery queues once in the base config previously, we were confusing things by appending to CELERY_QUEUES in both dev and test configs - these are executed at import time, so the list contained all queues twice, regardless of what config you're actually using. Fortunately, the -Q command that we supply the workers with overrides this config option, so other environments weren't affected. Given that, we can tidy up this code by just declaring it in the base config every time --- app/config.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/config.py b/app/config.py index ae7788d34..e61408ea3 100644 --- a/app/config.py +++ b/app/config.py @@ -270,7 +270,9 @@ class Config(object): 'options': {'queue': QueueNames.PERIODIC} } } - CELERY_QUEUES = [] + + # this is overriden by the -Q command, but locally, we should read from all queues + CELERY_QUEUES = [Queue(queue, Exchange('default'), routing_key=queue) for queue in QueueNames.all_queues()] NOTIFICATIONS_ALERT = 5 # five mins FROM_NUMBER = 'development' @@ -358,11 +360,6 @@ class Development(Config): STATSD_PORT = 1000 STATSD_PREFIX = "stats-prefix" - for queue in QueueNames.all_queues(): - Config.CELERY_QUEUES.append( - Queue(queue, Exchange('default'), routing_key=queue) - ) - API_HOST_NAME = "http://localhost:6011" API_RATE_LIMIT_ENABLED = True @@ -385,11 +382,6 @@ class Test(Development): BROKER_URL = 'you-forgot-to-mock-celery-in-your-tests://' - for queue in QueueNames.all_queues(): - Config.CELERY_QUEUES.append( - Queue(queue, Exchange('default'), routing_key=queue) - ) - API_RATE_LIMIT_ENABLED = True API_HOST_NAME = "http://localhost:6011" From e0551547d210354316621956fcda13e3d3ab388f Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 2 Jan 2018 10:28:16 +0000 Subject: [PATCH 3/7] wait 20 seconds between long polls --- app/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/config.py b/app/config.py index e61408ea3..c37dd5d00 100644 --- a/app/config.py +++ b/app/config.py @@ -150,9 +150,9 @@ class Config(object): BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { 'region': AWS_REGION, - 'polling_interval': 1, # 1 second 'visibility_timeout': 310, - 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX + 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX, + 'wait_time_seconds': 20 # enable long polling, with a wait time of 20 seconds } CELERY_ENABLE_UTC = True CELERY_TIMEZONE = 'Europe/London' From 6ca2b8277c4e20c7863d99cb9c6cc0f91e8079ad Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 3 Oct 2018 10:58:11 +0100 Subject: [PATCH 4/7] import exception from botocore boto (2) is no longer a dependency --- app/dao/notifications_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 95896e4da..6cfca54a8 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -5,7 +5,7 @@ from datetime import ( timedelta, ) -from boto.exception import BotoClientError +from botocore.exceptions import ClientError as BotoClientError from flask import current_app from notifications_utils.recipients import ( From 640f00b0e86a645d658ad0e718d4183844bd812a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 3 Oct 2018 11:57:13 +0100 Subject: [PATCH 5/7] install celery with sqs support you need to `pip install celery[sqs]` to get the additional dependencies that celery needs to use SQS queues - there are two libs - boto3 and pycurl. pycurl is a bunch of python handles around curl, so needs to be installed from source so it can link to your curl/ssl libs. On paas and in docker this works fine (needed to add `libcurl4-openssl-dev` to the docker container), but on macos it can't find openssl. We need to pass a couple of flags in: * set the environment variable PYCURL_SSL_LIBRARY=openssl * pass in the global options `build_ext` and `-I{openssl_headers_path}`. As shown here: https://github.com/pycurl/pycurl/issues/530#issuecomment-395403253 Env var is no biggie, but using any install-option flags disables wheels for the whole pip install run. (See https://github.com/pypa/pip/issues/2677 and https://github.com/pypa/pip/issues/4118 for more context on the install-options flags). A whole bunch of our dependencies don't install nicely from source (but do from wheel), so this commit installs pycurl separately as an initial step, with the requisite flags, and then installs the rest of the requirements as before. I've updated the makefile and bootstrap.sh files to reflect this, but if you run `pip install -r requirements.txt` from scratch you will run into issues. --- Makefile | 11 +++++++++++ docker/Dockerfile | 1 + requirements-app.txt | 2 +- requirements.txt | 5 +++-- scripts/bootstrap.sh | 1 + 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 3612540bc..4bf5f86d8 100644 --- a/Makefile +++ b/Makefile @@ -67,6 +67,7 @@ production: ## Set environment to production .PHONY: dependencies dependencies: venv ## Install build dependencies mkdir -p ${PIP_ACCEL_CACHE} + . venv/bin/activate && make install-pycurl . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements_for_test.txt .PHONY: generate-version-file @@ -97,6 +98,7 @@ test: venv generate-version-file ## Run tests freeze-requirements: rm -rf venv-freeze virtualenv -p python3 venv-freeze + $(call install-pycurl, $$(pwd)/venv-freeze/bin/pip) $$(pwd)/venv-freeze/bin/pip install -r requirements-app.txt echo '# pyup: ignore file' > requirements.txt echo '# This file is autogenerated. Do not edit it manually.' >> requirements.txt @@ -105,6 +107,15 @@ freeze-requirements: $$(pwd)/venv-freeze/bin/pip freeze -r <(sed '/^--/d' requirements-app.txt) | sed -n '/The following requirements were added by pip freeze/,$$p' >> requirements.txt rm -rf venv-freeze +define install-pycurl + # install pycurl separately to avoid flags disabling wheels for other packages + PYCURL_SSL_LIBRARY=openssl ${1} install pycurl --global-option="build_ext" --global-option="-I/usr/local/opt/openssl/include" +endef + +.PHONY: install-pycurl +install-pycurl: + $(call install-pycurl, pip) + .PHONY: test-requirements test-requirements: @diff requirements-app.txt requirements.txt | grep '<' \ diff --git a/docker/Dockerfile b/docker/Dockerfile index 1060824cf..db1bc6202 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,6 +22,7 @@ RUN \ libffi-dev \ python-dev \ jq \ + libcurl4-openssl-dev \ && echo "Clean up" \ && rm -rf /var/lib/apt/lists/* /tmp/* diff --git a/requirements-app.txt b/requirements-app.txt index dc9f43f31..373af0f75 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -2,7 +2,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery==4.2.1 +celery[sqs]==4.2.1 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 diff --git a/requirements.txt b/requirements.txt index 2a2622a60..00c37c4e3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery==4.2.1 +celery[sqs]==4.2.1 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 @@ -37,7 +37,7 @@ amqp==2.3.2 bcrypt==3.1.4 billiard==3.5.0.4 bleach==2.1.3 -boto3==1.6.16 +boto3==1.9.16 certifi==2018.8.24 chardet==3.0.4 Click==7.0 @@ -60,6 +60,7 @@ orderedset==2.0.1 phonenumbers==8.9.4 pyasn1==0.4.4 pycparser==2.19 +pycurl==7.43.0.2 PyPDF2==1.26.0 python-dateutil==2.7.3 python-editor==1.0.3 diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index c46d03aa6..e05501137 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -28,6 +28,7 @@ fi # we need the version file to exist otherwise the app will blow up make generate-version-file +make install-pycurl # Install Python development dependencies pip3 install -r requirements_for_test.txt From bfc4343b0e1310e3769168fddad33da2d6fd177e Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 4 Oct 2018 15:52:51 +0100 Subject: [PATCH 6/7] remove pip-accel and make sure commands work if you're in a venv remove pip-accel - it's not been updated in two years, and pins our version of pip to a version that is several breaking changes old. make sure commands work if you're already in a venv - mostly by checking for presence of $VIRTUAL_ENV, and ensuring we use the correct pip to install packages. Also clean up the commands a bit. --- Makefile | 26 +++++++++++++++----------- requirements-app.txt | 2 ++ scripts/bootstrap.sh | 12 +----------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/Makefile b/Makefile index 4bf5f86d8..e94889cea 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,6 @@ SHELL := /bin/bash DATE = $(shell date +%Y-%m-%d:%H:%M:%S) -PIP_ACCEL_CACHE ?= ${CURDIR}/cache/pip-accel APP_VERSION_FILE = app/version.py GIT_BRANCH ?= $(shell git symbolic-ref --short HEAD 2> /dev/null || echo "detached") @@ -34,11 +33,19 @@ help: @cat $(MAKEFILE_LIST) | grep -E '^[a-zA-Z_-]+:.*?## .*$$' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' .PHONY: venv +# if there's a virtualenv already active, we don't want to do anything +ifeq ($(VIRTUAL_ENV),) venv: venv/bin/activate ## Create virtualenv if it does not exist venv/bin/activate: test -d venv || virtualenv venv -p python3 - . venv/bin/activate && pip install pip-accel + +VENV_BIN_DIR = $(shell . venv/bin/activate && echo $$VIRTUAL_ENV/bin) +else +venv: + +VENV_BIN_DIR = $(VIRTUAL_ENV)/bin +endif .PHONY: check-env-vars check-env-vars: ## Check mandatory environment variables @@ -65,10 +72,9 @@ production: ## Set environment to production @true .PHONY: dependencies -dependencies: venv ## Install build dependencies - mkdir -p ${PIP_ACCEL_CACHE} - . venv/bin/activate && make install-pycurl - . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements_for_test.txt +dependencies: venv + $(call install-pycurl, $(VENV_BIN_DIR)/pip) + $(VENV_BIN_DIR)/pip install -r requirements_for_test.txt .PHONY: generate-version-file generate-version-file: ## Generates the app version file @@ -76,7 +82,7 @@ generate-version-file: ## Generates the app version file .PHONY: build build: dependencies generate-version-file ## Build project - . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements.txt + .PHONY: build-paas-artifact build-paas-artifact: ## Build the deploy artifact for PaaS @@ -109,7 +115,7 @@ freeze-requirements: define install-pycurl # install pycurl separately to avoid flags disabling wheels for other packages - PYCURL_SSL_LIBRARY=openssl ${1} install pycurl --global-option="build_ext" --global-option="-I/usr/local/opt/openssl/include" + PYCURL_SSL_LIBRARY=openssl ${1} install pycurl==7.43.0.2 --global-option="build_ext" --global-option="-I/usr/local/opt/openssl/include" endef .PHONY: install-pycurl @@ -125,11 +131,10 @@ test-requirements: .PHONY: coverage coverage: venv ## Create coverage report - . venv/bin/activate && coveralls + coveralls .PHONY: prepare-docker-build-image prepare-docker-build-image: ## Prepare the Docker builder image - mkdir -p ${PIP_ACCEL_CACHE} make -C docker build .PHONY: build-with-docker @@ -137,7 +142,6 @@ build-with-docker: prepare-docker-build-image ## Build inside a Docker container @docker run -i${DOCKER_TTY} --rm \ --name "${DOCKER_CONTAINER_PREFIX}-build" \ -v "`pwd`:/var/project" \ - -v "${PIP_ACCEL_CACHE}:/var/project/cache/pip-accel" \ -e UID=$(shell id -u) \ -e GID=$(shell id -g) \ -e GIT_COMMIT=${GIT_COMMIT} \ diff --git a/requirements-app.txt b/requirements-app.txt index 373af0f75..723a9b78c 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -28,3 +28,5 @@ awscli-cwlogs>=1.4,<1.5 botocore<1.11.0 git+https://github.com/alphagov/notifications-utils.git@30.4.0#egg=notifications-utils==30.4.0 + +# if you want to update pycurl please do so in makefile diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index e05501137..b92832233 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -20,17 +20,7 @@ function display_result { fi } -if [ ! $VIRTUAL_ENV ]; then - virtualenv -p python3 ./venv - . ./venv/bin/activate -fi - -# we need the version file to exist otherwise the app will blow up -make generate-version-file - -make install-pycurl -# Install Python development dependencies -pip3 install -r requirements_for_test.txt +make build # Create Postgres databases createdb notification_api From 2edb7f39e67202dd5316007a0b32d0c6f242ab23 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 5 Oct 2018 10:28:12 +0100 Subject: [PATCH 7/7] fix reqs file --- requirements.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 00c37c4e3..3bd1ea2f7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -31,13 +31,15 @@ botocore<1.11.0 git+https://github.com/alphagov/notifications-utils.git@30.4.0#egg=notifications-utils==30.4.0 +# if you want to update pycurl please do so in makefile + ## The following requirements were added by pip freeze: alembic==1.0.0 amqp==2.3.2 bcrypt==3.1.4 billiard==3.5.0.4 bleach==2.1.3 -boto3==1.9.16 +boto3==1.9.17 certifi==2018.8.24 chardet==3.0.4 Click==7.0