launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/geoip2 into lp:launchpad with lp:~cjwatson/launchpad/geoip-country as a prerequisite.

Commit message:
Add optional geoip2 support.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/geoip2/+merge/371095

We don't have a useful way to test this until the relevant data packages gain the right set of database files, but I've run the existing tests with modified configuration to point to a local download of GeoLite2-Country.mmdb and they pass.

Once we're using this, it will give us GeoIP support for IPv6 addresses.

https://code.launchpad.net/~cjwatson/lp-source-dependencies/+git/lp-source-dependencies/+merge/371094 and probably https://code.launchpad.net/~cjwatson/meta-lp-deps/geoip2/+merge/371092 should land first.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/geoip2 into lp:launchpad.

=== modified file 'constraints.txt'
--- constraints.txt	2019-07-09 12:32:22 +0000
+++ constraints.txt	2019-08-08 15:23:22 +0000
@@ -262,6 +262,7 @@
 fixtures==3.0.0
 FormEncode==1.2.4
 futures==3.2.0
+geoip2==2.9.0
 grokcore.component==1.6
 gunicorn==19.8.1
 html5browser==0.0.9
@@ -299,6 +300,7 @@
 manuel==1.7.2
 Markdown==2.3.1
 martian==0.11
+maxminddb==1.4.1
 meliae==0.2.0.final.0
 mistune==0.8.3
 mock==1.0.1

=== modified file 'lib/lp/services/geoip/model.py'
--- lib/lp/services/geoip/model.py	2019-08-08 15:22:41 +0000
+++ lib/lp/services/geoip/model.py	2019-08-08 15:23:22 +0000
@@ -10,6 +10,8 @@
 import os
 
 import GeoIP as libGeoIP
+from geoip2.database import Reader
+from geoip2.errors import AddressNotFoundError
 from zope.component import getUtility
 from zope.i18n.interfaces import IUserPreferredLanguages
 from zope.interface import implementer
@@ -38,20 +40,29 @@
         if not os.path.exists(config.launchpad.geoip_database):
             raise NoGeoIPDatabaseFound(
                 "No GeoIP DB found. Please install launchpad-dependencies.")
-        return libGeoIP.open(
-            config.launchpad.geoip_database, libGeoIP.GEOIP_MEMORY_CACHE)
+        if config.launchpad.geoip_database.endswith('.mmdb'):
+            return Reader(config.launchpad.geoip_database)
+        else:
+            return libGeoIP.open(
+                config.launchpad.geoip_database, libGeoIP.GEOIP_MEMORY_CACHE)
 
     def getCountryCodeByAddr(self, ip_address):
         """See `IGeoIP`."""
         if not ipaddress_is_global(ip_address):
             return None
-        try:
-            return self._gi.country_code_by_addr(ip_address)
-        except SystemError:
-            # libGeoIP may raise a SystemError if it doesn't find a record for
-            # some IP addresses (e.g. 255.255.255.255), so we need to catch
-            # that and return None here.
-            return None
+        if isinstance(self._gi, Reader):
+            try:
+                return self._gi.country(ip_address).country.iso_code
+            except AddressNotFoundError:
+                return None
+        else:
+            try:
+                return self._gi.country_code_by_addr(ip_address)
+            except SystemError:
+                # libGeoIP may raise a SystemError if it doesn't find a
+                # record for some IP addresses (e.g. 255.255.255.255), so we
+                # need to catch that and return None here.
+                return None
 
     def getCountryByAddr(self, ip_address):
         """See `IGeoIP`."""

=== modified file 'setup.py'
--- setup.py	2019-08-08 15:22:41 +0000
+++ setup.py	2019-08-08 15:23:22 +0000
@@ -161,6 +161,7 @@
         'feedparser',
         'feedvalidator',
         'fixtures',
+        'geoip2',
         'gunicorn[gthread]',
         'html5browser',
         'importlib-resources',