Opened 3 years ago
Closed 4 weeks 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 , 3 years ago
Description: | modified (diff) |
---|
comment:2 by , 3 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → New feature |
Version: | 4.0 → dev |
comment:3 by , 2 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 2 years ago
Needs documentation: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:6 by , 4 weeks 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 , 4 weeks ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Probably broken by 40b5b1596f7505416bd30d5d7582b5a9004ea7d5
comment:8 by , 4 weeks ago
Yes, quite likely. Unfortunately 1e5bbbb2a866faf56e00eb744ab940e40e87d497 didn't include any tests. Am looking at a fix for this now.
comment:10 by , 4 weeks ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:11 by , 4 weeks ago
Has patch: | unset |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:13 by , 4 weeks 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.