Opened 4 years ago
Closed 13 months ago
#33442 closed New feature (fixed)
contrib.gis.geoip does not support Country-Lite version from DBIP
| Reported by: | Nathan Humphreys | Owned by: | Nick Pope |
|---|---|---|---|
| Component: | GIS | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The
db_type
property of the geoip database metadata for the dbip (https://db-ip.com/db/lite.php) lite databases is being returned as "DBIP-Country-Lite"
The check here then fails
https://github.com/django/django/blob/stable/3.2.x/django/contrib/gis/geoip2/base.py#L107
despite the database being in the mmdb format.
I believe Django should support the (free) dbip lite databases as well as the commercial maxmind databases.
Changing the above line to check for
if db_type.contains("Country"):
...
would be sufficient. As would a second check to check for
if db_type.endswith("Country") or db_type.endswith("Country-Lite"):
...
A similar change would be required the the dbip City-Lite database to be supported.
I am happy to put in a pull request but I am also hoping we can find a way to get this into the 3.2.x branch as well as a future 4.x release if thats possible.
Change History (13)
comment:1 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 4 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Bug → New feature |
| Version: | 4.0 → dev |
comment:3 by , 3 years ago
| Has patch: | set |
|---|---|
| Needs documentation: | set |
| Owner: | changed from to |
| Status: | new → assigned |
comment:4 by , 3 years ago
| Needs documentation: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:6 by , 13 months ago
Fix appears to be reverted in 5.1.2
I get the following error:
django.contrib.gis.geoip2.GeoIP2Exception: Unable to handle database edition: DBIP-City-Lite
Unless i manually make the changes this ticket suggests.
comment:7 by , 13 months ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
Probably broken by 40b5b1596f7505416bd30d5d7582b5a9004ea7d5
comment:8 by , 13 months ago
Yes, quite likely. Unfortunately 1e5bbbb2a866faf56e00eb744ab940e40e87d497 didn't include any tests. Am looking at a fix for this now.
comment:10 by , 13 months ago
| Severity: | Normal → Release blocker |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
comment:11 by , 13 months ago
| Has patch: | unset |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:13 by , 13 months ago
| Has patch: | set |
|---|---|
| Resolution: | → fixed |
| Severity: | Release blocker → Normal |
| Status: | assigned → closed |
| Triage Stage: | Accepted → Ready for checkin |
Moved regression to ticket #35841.
A patch is welcome. Unfortunately, per Django release policy, this will not enter 3.2.x, nor 4.0.x, as it's not a regression.