From 75940c9566c09a2633dd700eb2d5f75b648a0f7a Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Tue, 10 Jul 2018 14:50:30 +0100 Subject: [PATCH] Pin all application requirements in requirements.txt The list of top-level dependencies is moved to requirements-app.txt, which is used by `make freeze-requirements` to generate the full list of requirements in requirements.txt. This is based on alphagov/digitalmarketplace-api#615, so rationale from that PR applies here. We had a problem with unpinned packages on new deployments leading to failed tests (e.g. alphagov/notifications-admin#2144) which is why we're implementing this now. After re-evaluating pipenv again, this still seems like the least disruptive approach: * pyup.io has experimental support for Pipfile, but doesn't respect version ranges or updating hashes in the lock file * CloudFoundry buildpack recognizes and supports Pipfiles out of the box, but the support is relatively new. For example until recently CF would install dev packages during deployment. It's also based on generating a requirements file from the Pipfile, which doesn't properly support pinning VCS dependencies (eg it doesn't set the #egg= version, meaning pip will not upgrade the package if it's already installed). * pipenv has a strict dependency resolution algorithm, which doesn't appear to be well documented and can cause some unexpected failures. For example, pipenv doesn't seem to be able to install `awscli-cwlogs` package at all, believing it to have a version conflict for `botocore` (which it doesn't list as a direct dependency) while neither `pip` nor `pip-tools` highlight any issues with it. * While trying out `pipenv install` on our list of dependencies it would regularly fail to install utils with a "Will try again." message. While the installation succeeds after a retry, this doesn't inspire confidence. * The switch to Pipfile and pipenv-managed virtualenvs requires a series of changes to `make` targets and scripts - replacing `pip install` with `pipenv`, removing references to requirements files and prefixing commands with `pipenv run`. While it's likely to simplify the overall process of managing dependencies, it would require time to properly implement across our applications and environments (Jenkins, PaaS, docker containers, and dev machines). --- Makefile | 18 +++++++++++++++ README.md | 13 +++++++++++ requirements-app.txt | 30 ++++++++++++++++++++++++ requirements.txt | 55 +++++++++++++++++++++++++++++++++++++++++++- scripts/run_tests.sh | 4 ++++ 5 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 requirements-app.txt diff --git a/Makefile b/Makefile index ebf56145a..5954b4705 100644 --- a/Makefile +++ b/Makefile @@ -93,6 +93,24 @@ upload-paas-artifact: test: venv generate-version-file ## Run tests ./scripts/run_tests.sh +.PHONY: freeze-requirements +freeze-requirements: + rm -rf venv-freeze + virtualenv -p python3 venv-freeze + $$(pwd)/venv-freeze/bin/pip install -r requirements-app.txt + echo '# This file is autogenerated. Do not edit it manually.' > requirements.txt + cat requirements-app.txt >> requirements.txt + echo '' >> requirements.txt + $$(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 + +.PHONY: test-requirements +test-requirements: + @diff requirements-app.txt requirements.txt | grep '<' \ + && { echo "requirements.txt doesn't match requirements-app.txt."; \ + echo "Run 'make freeze-requirements' to update."; exit 1; } \ +|| { echo "requirements.txt is up to date"; exit 0; } + .PHONY: coverage coverage: venv ## Create coverage report . venv/bin/activate && coveralls diff --git a/README.md b/README.md index 11db3cb26..810718d97 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,19 @@ That will run flake8 for code analysis and our unit test suite. If you wish to r [notifications-functional-tests](https://github.com/alphagov/notifications-functional-tests) repository. +## To update application dependencies + +`requirements.txt` file is generated from the `requirements-app.txt` in order to pin +versions of all nested dependencies. If `requirements-app.txt` has been changed (or +we want to update the unpinned nested dependencies) `requirements.txt` should be +regenerated with + +``` +make freeze-requirements +``` + +`requirements.txt` should be committed alongside `requirements-app.txt` changes. + ## To run one off tasks diff --git a/requirements-app.txt b/requirements-app.txt new file mode 100644 index 000000000..264f568f7 --- /dev/null +++ b/requirements-app.txt @@ -0,0 +1,30 @@ +# Run `make freeze-requirements` to update requirements.txt +# with package version changes made in requirements-app.txt + +cffi==1.11.5 +celery==3.1.26.post2 # pyup: <4 +docopt==0.6.2 +Flask-Bcrypt==0.7.1 +flask-marshmallow==0.8.0 +Flask-Migrate==2.2.1 +Flask-SQLAlchemy==2.3.2 +Flask==1.0.2 +click-datetime==0.2 +eventlet==0.23.0 +gunicorn==19.7.1 +iso8601==0.1.12 +jsonschema==2.6.0 +marshmallow-sqlalchemy==0.14.0 +marshmallow==2.15.3 +psycopg2-binary==2.7.5 +PyJWT==1.6.4 +SQLAlchemy==1.2.9 + +notifications-python-client==4.8.2 + +# PaaS +awscli-cwlogs>=1.4,<1.5 + +git+https://github.com/alphagov/notifications-utils.git@29.3.0#egg=notifications-utils==29.3.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 ea8ac1aa1..b9c187119 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,12 @@ +# This file is autogenerated. Do not edit it manually. +# Run `make freeze-requirements` to update requirements.txt +# with package version changes made in requirements-app.txt + cffi==1.11.5 celery==3.1.26.post2 # pyup: <4 docopt==0.6.2 Flask-Bcrypt==0.7.1 -Flask-Marshmallow==0.8.0 +flask-marshmallow==0.8.0 Flask-Migrate==2.2.1 Flask-SQLAlchemy==2.3.2 Flask==1.0.2 @@ -25,3 +29,52 @@ awscli-cwlogs>=1.4,<1.5 git+https://github.com/alphagov/notifications-utils.git@29.3.0#egg=notifications-utils==29.3.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==0.9.10 +amqp==1.4.9 +anyjson==0.3.3 +awscli==1.15.55 +bcrypt==3.1.4 +billiard==3.3.0.23 +bleach==2.1.3 +boto3==1.6.16 +botocore==1.10.54 +certifi==2018.4.16 +chardet==3.0.4 +click==6.7 +colorama==0.3.9 +docutils==0.14 +Flask-Redis==0.3.0 +future==0.16.0 +greenlet==0.4.13 +html5lib==1.0.1 +idna==2.7 +itsdangerous==0.24 +Jinja2==2.10 +jmespath==0.9.3 +kombu==3.0.37 +Mako==1.0.7 +MarkupSafe==1.0 +mistune==0.8.3 +monotonic==1.5 +orderedset==2.0.1 +phonenumbers==8.9.4 +pyasn1==0.4.3 +pycparser==2.18 +PyPDF2==1.26.0 +python-dateutil==2.7.3 +python-editor==1.0.3 +python-json-logger==0.1.8 +pytz==2018.5 +PyYAML==3.12 +redis==2.10.6 +requests==2.19.1 +rsa==3.4.2 +s3transfer==0.1.13 +six==1.11.0 +smartypants==2.0.1 +statsd==3.2.2 +urllib3==1.23 +webencodings==0.5.1 +Werkzeug==0.14.1 diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index d37db4608..d5480a3bd 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -23,6 +23,10 @@ function display_result { if [[ -z "$VIRTUAL_ENV" ]] && [[ -d venv ]]; then source ./venv/bin/activate fi + +make test-requirements +display_result $? 1 "Requirements check" + flake8 . display_result $? 1 "Code style check"