Incompatible pinned requirements

[tjd2002]

Both requirements.txt and requirements-dev.txt include
requests=2.20.0
urllib=1.25.2

This leads to the following conflict:

$ pip install -r requirements.txt -r requirements-dev.txt
[...]
ERROR: requests 2.20.0 has requirement urllib3<1.25,>=1.21.1, but you'll have urllib3 1.25.2 which is incompatible.

Pip actually completes the install, but we should choose compatible pinned versions. Also, should add a call to pip check to CI so that we capture problems like this.

[bendichter]

Thanks for the report, Tom. urllib3 was updated for security reasons. the requests library should support the new version of urllib3. They are aware of the issue and planning on rolling out an upgrade for urllib3 1.25 soon (https://github.com/kennethreitz/requests/issues/5065). I think the best move is to wait for them.

[t-b]

How about we say that -dev is an addition to the normal requirements file only? In that way there is no duplication.

[bendichter]

@t-b seems reasonable. @jcfr, thoughts?

[tjd2002]

I think we should add pip check to the tests to catch this situation in future. pip is quite happy to install conflicting packages (it reports the error, but then returns successfully).

[tjd2002]

@t-b sure--I believe that's what is currently done with the requirements-doc.txt file

[bendichter]

@tjd2002 yeah pip check sounds like a good idea

[tjd2002]

@bendichter upon reading your link more closely: it looks like the security fix was backported to urllib3==1.24.3, so I think that's what we should use until requests updates. I'll push a PR for that.

I'll also add pip check in a separate PR

[tjd2002]

Actually @t-b, adding in -r requirements.txt to the requirements-dev.txt file makes it so you can't pass that file into conda to create a new environment (as I like to do instead of using pip).

Also, the instructions for generating the requirements files currently recommend a nice method of just using pip freeze to capture all the pinned requirements.

Neither pip nor conda complains if there are exact duplicate lines across these files, so I think it's harmless to leave it with duplicate entries across these two files (at least for requirements-dev.txt. requirements-doc.txt does not have pinned versions, and looks like it has been manually managed, so I won't touch that for now).

[jcfr]

suggested next step

Waiting requests is updated to work with the fixed version of urllib3, I suggest to revert #914

pip check

should add a call to pip check to CI so that we capture problems like this.

Adding pip check makes sense, this page should also be updated. See https://pynwb.readthedocs.io/en/stable/update_requirements.html

requirements files

How about we say that -dev is an addition to the normal requirements file only? In that way there is no duplication

@t-b seems reasonable. @jcfr, thoughts?

The requirements files requirements.txt, requirements-dev.txt and requirements-doc.txt are all expected to contain the list of pinned dependencies (so that we have a reproducible environment for testing). The process to update these is documented here: https://pynwb.readthedocs.io/en/stable/update_requirements.html

They contain duplicated packages due to the steps currently documented for updating them.

On the other hand, the dependency specified in setup.py do not have bound and are extracted from requirements.txt for convenience. See

pynwb/setup.py

Lines 16 to 18 in 765aeb7

	reqs_re = re.compile("[<=>]+")
	with open('requirements.txt', 'r') as fp:
	reqs = [reqs_re.split(x.strip())[0] for x in fp.readlines()]

[tjd2002]

@jcfr

suggested next step

Waiting requests is updated to work with the fixed version of urllib3, I suggest to revert #914

The security issue CVE-2019-9740 has been backported to the 1.24 series of urllib3 (see release notes section at https://pypi.org/project/urllib3/1.24.3/ ), so I think it is should be safe to use 1.24.3. This is what I did in #923.

Adding pip check makes sense, this page should also be updated. See https://pynwb.readthedocs.io/en/stable/update_requirements.html

OK this is done in #925. Adds pip check to tox, and adds a recommendation to the docs to use pip check before updating the requirements.