launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Switch to the GeoIP country database.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/geoip-country/+merge/371044

The only piece of information we use from this now is the country code, so there's no point reading the much larger city database.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/geoip-country into lp:launchpad.

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2019-06-18 17:30:52 +0000
+++ configs/development/launchpad-lazr.conf	2019-08-07 15:43:20 +0000
@@ -109,7 +109,6 @@
 summary_list_size: 5
 max_bug_feed_cache_minutes: 30
 bzr_imports_root_url: file:///tmp/bazaar-branches
-geoip_database: /usr/share/GeoIP/GeoLiteCity.dat
 feature_flags_endpoint: http://xmlrpc-private.launchpad.test:8087/featureflags/
 
 [launchpad_session]

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2019-05-22 14:57:45 +0000
+++ configs/testrunner/launchpad-lazr.conf	2019-08-07 15:43:20 +0000
@@ -103,7 +103,6 @@
 [launchpad]
 basic_auth_password: test
 max_attachment_size: 1024
-geoip_database: /usr/share/GeoIP/GeoLiteCity.dat
 logparser_max_parsed_lines: 100000
 homepage_recent_posts_feed: http://launchpad.test:8093/blog-feed
 openid_canonical_root: http://testopenid.test/

=== modified file 'lib/lp/services/apachelogparser/base.py'
--- lib/lp/services/apachelogparser/base.py	2019-07-25 15:00:18 +0000
+++ lib/lp/services/apachelogparser/base.py	2019-08-07 15:43:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from datetime import datetime
@@ -148,10 +148,7 @@
                 file_downloads[day] = {}
             daily_downloads = file_downloads[day]
 
-            country_code = None
-            geoip_record = geoip.getRecordByAddress(host)
-            if geoip_record is not None:
-                country_code = geoip_record['country_code']
+            country_code = geoip.getCountryCodeByAddr(host)
             if country_code not in daily_downloads:
                 daily_downloads[country_code] = 0
             daily_downloads[country_code] += 1

=== modified file 'lib/lp/services/apachelogparser/tests/apache-log-files/launchpadlibrarian.net.access-log'
--- lib/lp/services/apachelogparser/tests/apache-log-files/launchpadlibrarian.net.access-log	2010-06-17 15:23:40 +0000
+++ lib/lp/services/apachelogparser/tests/apache-log-files/launchpadlibrarian.net.access-log	2019-08-07 15:43:20 +0000
@@ -2,5 +2,5 @@
 121.44.28.210 - - [13/Jun/2008:14:55:06 +0100] "GET /9096290/me-tv-icon-14x14.png HTTP/1.1" 200 730 "https://launchpad.net/me-tv"; "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060900 Firefox/3.0"
 121.44.28.210 - - [13/Jun/2008:14:55:13 +0100] "GET /12060796/me-tv-icon-64x64.png HTTP/1.1" 200 6378 "https://launchpad.net/me-tv"; "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060900 Firefox/3.0"
 157.92.18.21 - - [13/Jun/2008:14:55:15 +0100] "GET /8196569/mediumubuntulogo.png HTTP/1.1" 200 3420 "https://bugs.launchpad.net/ubuntu/+source/acpi-support/+bug/59695/comments/14"; "Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9) Gecko/2008052623 Iceweasel/3.0 (Debian-3.0~rc1-1)"
-210.79.178.243 - - [13/Jun/2008:14:55:21 +0100] "GET /8196569/mediumubuntulogo.png HTTP/1.1" 200 3420 "https://launchpad.net/ubuntu/hardy/hppa/xserver-xorg-driver-savage"; "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060309 Firefox/3.0"
+202.214.194.147 - - [13/Jun/2008:14:55:21 +0100] "GET /8196569/mediumubuntulogo.png HTTP/1.1" 200 3420 "https://launchpad.net/ubuntu/hardy/hppa/xserver-xorg-driver-savage"; "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060309 Firefox/3.0"
 69.233.136.42 - - [13/Jun/2008:14:55:22 +0100] "GET /15018215/ul_logo_64x64.png HTTP/1.1" 200 2261 "https://launchpad.net/~ubuntulite/+archive"; "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5) Gecko/2008041514 Firefox/3.0b5"

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2019-07-11 13:27:09 +0000
+++ lib/lp/services/config/schema-lazr.conf	2019-08-07 15:43:20 +0000
@@ -1112,7 +1112,7 @@
 code_homepage_product_cloud_size: 120
 
 # The database used by our GeoIP library.
-geoip_database: /usr/share/GeoIP/GeoIPCity.dat
+geoip_database: /usr/share/GeoIP/GeoIP.dat
 
 # The maximum number of lines that should be parsed by the launchpad
 # log parser. The default value of None means there is no maximum.

=== modified file 'lib/lp/services/geoip/doc/geoip.txt'
--- lib/lp/services/geoip/doc/geoip.txt	2019-08-07 15:42:37 +0000
+++ lib/lp/services/geoip/doc/geoip.txt	2019-08-07 15:43:20 +0000
@@ -37,32 +37,19 @@
     >>> print(geoip.getCountryByAddr('fc00::1'))
     None
 
-IGeoIP also provides a getRecordByAddress() method, which returns a dict
-containing a bunch more information about that IP's location.
+IGeoIP also provides a getCountryCodeByAddr() method, which returns just the
+country code without looking it up in the database.
 
-    >>> record = geoip.getRecordByAddress('201.13.165.145')
-    >>> for key, value in sorted(record.items()):
-    ...     print "%s: %s" % (key, value)
-    area_code: ...
-    city: ...
-    country_code: BR
-    country_code3: BRA
-    country_name: Brazil
-    dma_code: ...
-    latitude: ...
-    longitude: ...
-    postal_code: ...
-    region: ...
-    region_name: ...
-    time_zone: ...
+    >>> print(geoip.getCountryCodeByAddr('201.13.165.145'))
+    BR
 
 And again we'll return None if the address is private.
 
-    >>> print(geoip.getRecordByAddress('127.0.0.1'))
+    >>> print(geoip.getCountryCodeByAddr('127.0.0.1'))
     None
 
 If it can't find a GeoIP record for the given IP address, it will return
 None.
 
-    >>> print geoip.getRecordByAddress('255.255.255.255')
+    >>> print(geoip.getCountryCodeByAddr('255.255.255.255'))
     None

=== modified file 'lib/lp/services/geoip/interfaces.py'
--- lib/lp/services/geoip/interfaces.py	2019-08-07 15:42:37 +0000
+++ lib/lp/services/geoip/interfaces.py	2019-08-07 15:43:20 +0000
@@ -14,8 +14,8 @@
 class IGeoIP(Interface):
     """The GeoIP utility, which represents the GeoIP database."""
 
-    def getRecordByAddress(ip_address):
-        """Return the GeoIP record for the given IP address, or None."""
+    def getCountryCodeByAddr(ip_address):
+        """Return the country code for the given IP address, or None."""
 
     def getCountryByAddr(ip_address):
         """Find and return an ICountry based on the given IP address.

=== modified file 'lib/lp/services/geoip/model.py'
--- lib/lp/services/geoip/model.py	2019-08-07 15:42:37 +0000
+++ lib/lp/services/geoip/model.py	2019-08-07 15:43:20 +0000
@@ -41,12 +41,12 @@
         return libGeoIP.open(
             config.launchpad.geoip_database, libGeoIP.GEOIP_MEMORY_CACHE)
 
-    def getRecordByAddress(self, ip_address):
+    def getCountryCodeByAddr(self, ip_address):
         """See `IGeoIP`."""
         if not ipaddress_is_global(ip_address):
             return None
         try:
-            return self._gi.record_by_addr(ip_address)
+            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
@@ -57,10 +57,9 @@
         """See `IGeoIP`."""
         if not ipaddress_is_global(ip_address):
             return None
-        geoip_record = self.getRecordByAddress(ip_address)
-        if geoip_record is None:
+        countrycode = self.getCountryCodeByAddr(ip_address)
+        if countrycode is None:
             return None
-        countrycode = geoip_record['country_code']
 
         countryset = getUtility(ICountrySet)
         try: