Add test to ensure app requirements are complete

[ghost]

In a recent release the app now requires importlib.metadata.

Currently this is installed in requirements_dev.txt but not in requirements.txt.

Without manually installing importlib-metadata the app errors on start up, as follows:

  File "/home/cove/cove/.ve/lib/python3.6/site-packages/bootstrap3/__init__.py", line 8, in <module>
    from importlib.metadata import metadata
ModuleNotFoundError: No module named 'importlib.metadata'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./cove_project/wsgi.py", line 16, in <module>
    application = get_wsgi_application()
  File "/home/cove/cove/.ve/lib/python3.6/site-packages/django/core/wsgi.py", line 12, in get_wsgi_application
    django.setup(set_prefix=False)
  File "/home/cove/cove/.ve/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/cove/cove/.ve/lib/python3.6/site-packages/django/apps/registry.py", line 91, in populate
    app_config = AppConfig.create(entry)
  File "/home/cove/cove/.ve/lib/python3.6/site-packages/django/apps/config.py", line 90, in create
    module = import_module(entry)
  File "/home/cove/cove/.ve/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/cove/cove/.ve/lib/python3.6/site-packages/bootstrap3/__init__.py", line 10, in <module>
    from importlib_metadata import metadata
ModuleNotFoundError: No module named 'importlib_metadata'

[jpmckinney]

Thanks, @RobHooper. I noticed the error during deployment, but didn't have the chance to fix it at the time. For this repo, I had to re-run pip-compile; pip-compile requirements_dev.in to get the correct requirements.

[jpmckinney]

Fixed in https://github.com/open-contracting/cove-oc4ids/commit/0827034552fe650aec878d7000ec55754786d02b

[jpmckinney]

This bug slipped through the tests, because the tests install requirements_dev.txt (which included importlib-metadata) in order to have access to pytest, etc.

If this error recurs, we could change the GitHub Actions workflow to first just install requirements.txt and attempt to run a Django management command, which will raise an error if any production requirements are missing.

[michaelwood]

Could also switch to using the layering method instead. https://pypi.org/project/pip-tools/ "Workflow for layered requirements"

diff --git a/requirements_dev.in b/requirements_dev.in
index 52353c6..744700f 100644
--- a/requirements_dev.in
+++ b/requirements_dev.in
@@ -1,4 +1,4 @@
--r requirements.txt
+-c requirements.txt
 pip-tools
 coveralls
 pytest
diff --git a/requirements_dev.txt b/requirements_dev.txt
index 6340793..fdad76c 100644
--- a/requirements_dev.txt
+++ b/requirements_dev.txt
@@ -4,82 +4,43 @@
 #
 #    pip-compile requirements_dev.in
 #
-attrs==20.1.0             # via -r requirements.txt, jsonschema, pytest
-bleach==3.1.5             # via -r requirements.txt, libcoveocds
-cached-property==1.5.1    # via -r requirements.txt, libcove, libcoveocds, libcoveweb
-certifi==2020.6.20        # via -r requirements.txt, requests, sentry-sdk
-chardet==3.0.4            # via -r requirements.txt, requests
-click==7.1.2              # via -r requirements.txt, libcoveocds, pip-tools
-commonmark==0.9.1         # via -r requirements.txt, libcove, libcoveocds
-contextlib2==0.6.0.post1  # via -r requirements.txt, schema
+attrs==20.1.0             # via -c requirements.txt, pytest
+certifi==2020.6.20        # via -c requirements.txt, requests
+chardet==3.0.4            # via -c requirements.txt, requests
+click==7.1.2              # via -c requirements.txt, pip-tools
 coverage==5.2.1           # via coveralls, pytest-cov
 coveralls==2.1.2          # via -r requirements_dev.in
-dealer==2.1.0             # via -r requirements.txt, libcoveweb
-defusedxml==0.6.0         # via -r requirements.txt, odfpy
-django-bootstrap3==14.1.0  # via -r requirements.txt, libcoveweb
-django-debug-toolbar==2.2  # via -r requirements.txt, libcoveweb
-django-environ==0.4.5     # via -r requirements.txt, libcoveweb
-django==2.2.16            # via -r requirements.txt, django-bootstrap3, django-debug-toolbar, libcoveoc4ids, libcoveocds, libcoveweb
 docopt==0.6.2             # via coveralls
-et-xmlfile==1.0.1         # via -r requirements.txt, openpyxl
 flake8==3.8.3             # via -r requirements_dev.in
-flattentool==0.15.0       # via -r requirements.txt, libcove, libcoveoc4ids
 gitdb==4.0.5              # via gitpython
 gitpython==3.1.7          # via transifex-client
-idna==2.10                # via -r requirements.txt, requests
-importlib-metadata==1.7.0  # via -r requirements.txt, django-bootstrap3, flake8, jsonschema, pluggy, pytest
+idna==2.10                # via -c requirements.txt, requests
 iniconfig==1.0.1          # via pytest
 isort==5.4.2              # via -r requirements_dev.in
-jdcal==1.4.1              # via -r requirements.txt, openpyxl
-json-merge-patch==0.2     # via -r requirements.txt, libcove, libcoveocds, libcoveweb
-jsonref==0.2              # via -r requirements.txt, flattentool, libcove, libcoveweb
-jsonschema==3.2.0         # via -r requirements.txt, libcove, libcoveweb
-lepl==5.1.3               # via -r requirements.txt, rfc6266
-libcove==0.20.0           # via -r requirements.txt, libcoveoc4ids, libcoveocds, libcoveweb
-libcoveoc4ids==0.3.1      # via -r requirements.txt
-libcoveocds==0.9.1        # via -r requirements.txt, libcoveoc4ids
-libcoveweb==0.18.0        # via -r requirements.txt
 libsass==0.20.1           # via -r requirements_dev.in
-lxml==4.5.2               # via -r requirements.txt, flattentool
 mccabe==0.6.1             # via flake8
 more-itertools==8.5.0     # via pytest
-odfpy==1.4.1              # via -r requirements.txt, flattentool
-openpyxl==2.6.4           # via -r requirements.txt, flattentool, libcoveweb
-packaging==20.4           # via -r requirements.txt, bleach, pytest
+packaging==20.4           # via -c requirements.txt, pytest
 pip-tools==5.3.1          # via -r requirements_dev.in
 pluggy==0.13.1            # via pytest
 py==1.9.0                 # via pytest
 pycodestyle==2.6.0        # via flake8
 pyflakes==2.2.0           # via flake8
-pyparsing==2.4.7          # via -r requirements.txt, packaging
-pyrsistent==0.16.0        # via -r requirements.txt, jsonschema
+pyparsing==2.4.7          # via -c requirements.txt, packaging
 pytest-cov==2.10.1        # via -r requirements_dev.in
 pytest-django==3.9.0      # via -r requirements_dev.in
 pytest-localserver==0.5.0  # via -r requirements_dev.in
 pytest==6.0.1             # via -r requirements_dev.in, pytest-cov, pytest-django
-python-dateutil==2.8.1    # via -r requirements.txt, libcoveweb
-python-memcached==1.59    # via -r requirements.txt
 python-slugify==4.0.1     # via transifex-client
-pytz==2020.1              # via -r requirements.txt, django, flattentool
-requests==2.24.0          # via -r requirements.txt, coveralls, libcove, libcoveocds, libcoveweb, transifex-client
-rfc3987==1.3.8            # via -r requirements.txt, libcove, libcoveoc4ids, libcoveweb
-rfc6266==0.0.4            # via -r requirements.txt, libcoveweb
-schema==0.7.2             # via -r requirements.txt, flattentool
+requests==2.24.0          # via -c requirements.txt, coveralls, transifex-client
 selenium==3.141.0         # via -r requirements_dev.in
-sentry-sdk==0.17.3        # via -r requirements.txt, libcoveweb
-six==1.15.0               # via -r requirements.txt, bleach, jsonschema, libsass, packaging, pip-tools, pyrsistent, python-dateutil, python-memcached, transifex-client
+six==1.15.0               # via -c requirements.txt, libsass, packaging, pip-tools, transifex-client
 smmap==3.0.4              # via gitdb
-sqlparse==0.3.1           # via -r requirements.txt, django, django-debug-toolbar
-strict-rfc3339==0.7       # via -r requirements.txt, libcove, libcoveoc4ids, libcoveweb
 text-unidecode==1.3       # via python-slugify
 toml==0.10.1              # via pytest
 transifex-client==0.13.11  # via -r requirements_dev.in
-urllib3==1.25.10          # via -r requirements.txt, requests, selenium, sentry-sdk, transifex-client
-webencodings==0.5.1       # via -r requirements.txt, bleach
+urllib3==1.25.10          # via -c requirements.txt, requests, selenium, transifex-client
 werkzeug==1.0.1           # via pytest-localserver
-xmltodict==0.12.0         # via -r requirements.txt, flattentool, libcoveweb
-zipp==1.2.0               # via -r requirements.txt, importlib-metadata, libcoveweb

 # The following packages are considered to be unsafe in a requirements file:
 # pip
-# setuptools

[jpmckinney]

@michaelwood We can do that. However, I'm not sure it resolves the issue. What did you run to generate requirements.txt in https://github.com/open-contracting/cove-oc4ids/commit/a304e58aaca50af062302e1ff5d25a8eef2e1ad6 ? I run the commands here, and they don't cause importlib-metadata to be dropped off: https://ocp-software-handbook.readthedocs.io/en/latest/python/applications.html#requirements

[jpmckinney]

Hmm, I don't think -c is the behavior we want. Small example based on pip-tools example. Input:

# requirements.in
django<2.3
requests

# dev-requirements.in
-c requirements.txt
django-debug-toolbar

Output:

# dev-requirements.txt
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile dev-requirements.in
#
django-debug-toolbar==3.1.1  # via -r dev-requirements.in
django==2.2.16            # via -c requirements.txt, django-debug-toolbar
pytz==2020.1              # via -c requirements.txt, django
sqlparse==0.4.1           # via -c requirements.txt, django, django-debug-toolbar

We want requests and dependencies to appear in dev-requirements.txt without either (1) having to repeat them in dev-requirements.in or (2) having to install dependencies for development with pip install -r requirements.txt -r dev-requirements.txt instead of pip install -r dev-requirements.txt.

Personally, I don't mind pip install -r requirements.txt -r requirements_dev.txt, so if -c solves some problems, we can switch to that.

[michaelwood]

Hmm yeah, just sort of thinking out loud. I also just wonder, if wasting 64MB of unused dev dependencies really matters, it isn't very computer science but If you're caring about 64MB of free space on a server you probably have other problems!

Comparing it to NodeJS where you can have any number of copies of the same library in a single project it is positively efficient!

[jpmckinney]

I had a look and most of our requirements_dev.in files are identical:

-r requirements.txt
coveralls
flake8
isort
pip-tools
pytest
pytest-cov

Separating requirements is a common practice (there's even a requirements-dev.txt package on PyPi to help people who forget to type -r). For packages, it definitely makes sense not to install dev dependencies on a user's system who doesn't need them. For applications, the reasons aren't as strong: make deploys faster, avoid any risk of dev dependencies changing behavior of other packages, require developers to be deliberate about which packages they import in application code versus test code, and the principle that only the minimum of what is needed ought to be deployed in production.

We haven't had issues before. We can easily cover this scenario by adding this to tests, before installing the requirements_dev.txt file. For Django apps:

- run: pip install -r requirements.txt
- run: ./manage.py --help

When a package like importlib-metadata is not installed, it raises ModuleNotFoundError.

[jpmckinney]

I forgot: A major reason for separating dev dependencies is that a maintainer can very confidently remove dev dependencies (as long as tests pass, no problem!), whereas a maintainer needs to be more careful when removing app dependencies (e.g. tests might still pass if coverage is low).

In my experience, I also see a tendency for requirements to balloon (even when no longer needed) when dev and app dependencies are mixed – partly for the above reason, where a maintainer might see a package in the requirements that they suspect is no longer needed, but it's too much effort to check.

To my knowledge, this issue has only been encountered by @michaelwood, and I'm not sure how it occurs, because using pip-compile; pip-compile requirements_dev.in as documented, has never caused a problem in my work.

So, I'm re-focusing the issue to be about adding this for Django apps, to protect against any such mistakes:

- run: pip install -r requirements.txt
- run: ./manage.py --help

[jpmckinney]

Done in all Django apps.