Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/team-members-ordering into lp:launchpad

2019-08-07 Thread William Grant
Review: Approve code


-- 
https://code.launchpad.net/~cjwatson/launchpad/team-members-ordering/+merge/371035
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/geoip-country into lp:launchpad

2019-08-07 Thread Colin Watson
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 +
+++ configs/development/launchpad-lazr.conf	2019-08-07 15:43:20 +
@@ -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 +
+++ configs/testrunner/launchpad-lazr.conf	2019-08-07 15:43:20 +
@@ -103,7 +103,6 @@
 [launchpad]
 basic_auth_password: test
 max_attachment_size: 1024
-geoip_database: /usr/share/GeoIP/GeoLiteCity.dat
 logparser_max_parsed_lines: 10
 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 +
+++ lib/lp/services/apachelogparser/base.py	2019-08-07 15:43:20 +
@@ -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 +
+++ lib/lp/services/apachelogparser/tests/apache-log-files/launchpadlibrarian.net.access-log	2019-08-07 15:43:20 +
@@ -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 +
+++ lib/lp/services/config/schema-lazr.conf	2019-08-07 15:43:20 +
@@ -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 

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/team-members-ordering into lp:launchpad

2019-08-07 Thread Colin Watson
Colin Watson has proposed merging lp:~cjwatson/launchpad/team-members-ordering 
into lp:launchpad.

Commit message:
Sort team members collections so that webservice batching works more reliably.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1839132 in Launchpad itself: "The API's people[groupname].members method 
can return duplicates"
  https://bugs.launchpad.net/launchpad/+bug/1839132

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/team-members-ordering/+merge/371035
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/team-members-ordering into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_person_webservice.py'
--- lib/lp/registry/browser/tests/test_person_webservice.py	2019-05-22 14:57:45 +
+++ lib/lp/registry/browser/tests/test_person_webservice.py	2019-08-07 08:38:58 +
@@ -1,10 +1,12 @@
-# Copyright 2009-2018 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).
 
 __metaclass__ = type
 
+from operator import attrgetter
 import textwrap
 
+from lazr.uri import URI
 from storm.store import Store
 from zope.component import getUtility
 from zope.security.management import endInteraction
@@ -189,6 +191,37 @@
 get_members, create_member, 2)
 self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
 
+def test_members_ordering(self):
+# Entries in the various members collections are sorted by
+# (Person.display_name, Person.name).
+with admin_logged_in():
+team = self.factory.makeTeam()
+owner = team.teamowner
+name = team.name
+members = [owner]
+with admin_logged_in():
+for member_suffix in (
+'a1', 'b1', 'a2', 'b2', 'a3', 'b4', 'a4', 'b3',
+'a5', 'b5'):
+person = self.factory.makePerson(
+name='member-' + member_suffix)
+team.addMember(person, owner)
+members.append(person)
+expected_member_names = [
+member.name for member in sorted(
+members, key=attrgetter('display_name', 'name'))]
+ws = webservice_for_person(owner)
+observed_member_names = []
+batch = ws.get('/~%s/members' % name).jsonBody()
+while True:
+for entry in batch['entries']:
+observed_member_names.append(entry['name'])
+next_link = batch.get('next_collection_link')
+if next_link is None:
+break
+batch = ws.get(URI(next_link)).jsonBody()
+self.assertEqual(expected_member_names, observed_member_names)
+
 def test_many_ppas(self):
 # POSTing to a person with many PPAs doesn't OOPS.
 with admin_logged_in():

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-11-13 03:48:13 +
+++ lib/lp/registry/model/person.py	2019-08-07 08:38:58 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 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).
 
 """Implementation classes for a Person."""
@@ -4040,6 +4040,12 @@
 # Adapt the result into a cached Person.
 columns = tuple(columns)
 raw_result = store.using(*origin).find(columns, conditions)
+if need_api:
+# Collections exported on the API need to be sorted in order to
+# provide stable batching.  In some ways Person.name might make
+# more sense, but we use the same ordering as in web UI views to
+# minimise confusion.
+raw_result = raw_result.order_by(Person.sortingColumns)
 
 def preload_for_people(rows):
 if need_teamowner or need_api:

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp