GeoIP2-python
GeoIP2-python copied to clipboard
Removing unused dependency, adding optional dependencies and loosening requests version
Linked to Issue #101. I noticed when installing the library that the dependency tree is quite deep. I have to install to a non-internet connected environment so I have to download all of the .whls manually which highlighted the issue.

I saw some opportunities for making the installation a bit leaner.
-
Have removed unused dependency
urllib3. -
Have made
aiohttpandrequestsoptional installs. User gets an error message when trying to import the libraries withoutrequestsoraiohttpinstalled
>>> import geoip2.webservice
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/sean/GeoIP2-python/geoip2/webservice.py", line 49, in <module>
raise ImportError("""To enable geoip2
ImportError: To enable geoip2.webservice, install aiohttp or requests support.
pip install geoip2[aiohttp]
pip install geoip2[requests]
pip install geoip2[all]
Some figures on the size of the install for different configurations
geoip2 - 0.3MB
geoip2[aiohttp] - 12.4MB
geoip2[requests] - 2.3MB
geoip2[all] - 14.4MB
This would be a big change to how this library is installed so this would need some maintainer feedback on this level of change.
-
Also I noticed a very specific version of
requestshas been picked>=2.24.0which is from June 2020 onwards. I'm not sure this package needs such a specific version looking the history of therequestspackage (https://github.com/psf/requests/blob/master/HISTORY.md). And it'll require the majority of people who use it who are already usingrequeststo need to uninstall their existing version and install 2.24.0.Since the minimum supported version of Python for this library is Python 3.6, and the earliest version of
requeststhat supports Python 3.6 is2.14.0I've suggested updating this dependency torequests>=2.14.0,<3.0.0. I've tested withrequests 2.14.0and all tests are passing with identical results.
Although the extra dependencies do make some sense, this would likely be a breaking change for many existing web service users. I don't know if this is easily resolvable with Setuptools. Perhaps at some point, it would make sense to break this up into several packages but it would be nice to that in such a way that doesn't increase maintenance burden.
I believe urllib3 is an indirect dependency and was added as a direct dependency to ensure that our users were not subject to a security issue with earlier versions. Similarly, the requests version specified in this PR has at least one significant security vulnerability. It has been our preference to require a recent version of these deps to make sure users who only depend on the libraries indirectly get security updates and fixes for other issues.
Although the extra dependencies do make some sense, this would likely be a breaking change for many existing web service users. I don't know if this is easily resolvable with Setuptools. Perhaps at some point, it would make sense to break this up into several packages but it would be nice to that in such a way that doesn't increase maintenance burden.
The extras-feature of setuptools is already a very easy way to "split" the requirements.
I believe
urllib3is an indirect dependency and was added as a direct dependency to ensure that our users were not subject to a security issue with earlier versions.
requests even comes with a built-in version of urllib3, so that won't be effective.
Similarly, the
requestsversion specified in this PR has at least one significant security vulnerability. It has been our preference to require a recent version of these deps to make sure users who only depend on the libraries indirectly get security updates and fixes for other issues.
Although distributions ship "older" versions of requests, they patch the security issues in the library. Requiring very current versions, causes an upgrade of requests during the installation of geoip2, possible causing compatibility issues. Could you instead recommend a higher version (e.g. using an optional requirements.txt) but not strictly requiring (in setup.py/cfg) it?