GeoIP2-python icon indicating copy to clipboard operation
GeoIP2-python copied to clipboard

Make `aiohttp` and `requests` optional dependencies

Open akx opened this issue 1 year ago • 7 comments

This PR makes aiohttp and requests, only required for geoip2.webservice, optional dependencies.

They can be installed separately (many projects will likely have requests anyway), or using the extras provided, i.e.

pip install GeoIP2[aiohttp,requests]

to use the version range specified by requirements.

The Tox configuration is adjusted so tests are run both with and without the libraries installed.

This fixes #101 (makes it possible to install without the webservice bits). This closes #104 (supersedes it to work with the current packaging and testing infrastructure).

akx avatar Aug 27 '24 07:08 akx

As discussed in the issue and other PR, this is a breaking change for web service users. I'd prefer either a solution that allows you to opt-out of the dependencies rather than opt-in or one that breaks the package into several packages where you could depend on the parts that you need with geoip2 remaining a meta packages that includes all of the parts.

oschwald avatar Aug 27 '24 15:08 oschwald

As discussed in the issue and other PR, this is a breaking change for web service users.

Yes, in semver terms this would be a major version bump. (In fact, the reason why I looked into this was that we have a downstream application that has pinned GeoIP to a much older version because the new version would pull in aiohttp and other things we don't want.)

However, practically, chances are that web service users will already have requests or aiohttp installed via other dependencies, or their application's direct dependencies.

I'd prefer either a solution that allows you to opt-out of the dependencies rather than opt-in

Unfortunately that's not possible with Python package extras.

one that breaks the package into several packages where you could depend on the parts that you need with geoip2 remaining a meta packages that includes all of the parts.

In my view, it would be significantly more packaging and infra work for little profit. However, this practically does that already: the core package is geoip2, the "package" that automagically supports sync web services is geoip2[requests], the "package" that automagically supports async web services is geoip2[aiohttp].

akx avatar Aug 28 '24 09:08 akx

Rebased on 5.0.1.

akx avatar Feb 10 '25 14:02 akx

Rebased.

Looking at the changelog for 5.0.0, this could easily be just another "BREAKING" change for 6.0.0?

akx avatar Aug 20 '25 07:08 akx

Thanks for the PR!

Looking at it, I think the approach still has the concerns @oschwald mentioned, in that I think this could break things for existing users as they would need to install new packages in situations where they are not already installing requests/aiohttp. Even when we have breaking changes, we'd prefer to make them as undisruptive as possible.

I understand the current situation isn't ideal. And there may not be a great alternative with the current packaging system. However I think the status quo is preferable until we have a solution that is less disruptive.

horgh avatar Aug 20 '25 19:08 horgh

In my opinion, asking users to change a geoip2 requirement to geoip2[requests] or geoip2[aiohttp] depending on what they're doing is not very disruptive 😄

For me and the company I work for, this is a supply chain vettability and possible security issue. As it is, installing geoip2 for any project pulls in all of these packages:

$ docker run -it python:3.13 pip install geoip2
[...]
aiohappyeyeballs-2.6.1 aiohttp-3.12.15 aiosignal-1.4.0 attrs-25.3.0 certifi-2025.8.3 
charset_normalizer-3.4.3 frozenlist-1.7.0 geoip2-5.1.0 idna-3.10 maxminddb-2.8.2 multidict-6.6.4 
propcache-0.3.2 requests-2.32.5 urllib3-2.5.0 yarl-1.20.1

While installing the library from this PR's HEAD (and in fact, this is what we currently do in production):

$ docker run -it python:3.13 pip install git+https://github.com/akx/GeoIP2-python@split-deps
[...]
Successfully installed geoip2-5.1.0 maxminddb-2.8.2

akx avatar Aug 21 '25 05:08 akx

I understand the concern, but we'd prefer to be conservative about potential breakage. The gain here doesn't seem big enough to warrant that.

horgh avatar Aug 21 '25 18:08 horgh