launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22364
[Merge] lp:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad.
Commit message:
Assorted sitesearch fixes and improvements.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-1/+merge/343131
Assorted sitesearch fixes and improvements.
- Rename BingSearchService._parse_bing_response() and GoogleSearchService._parse_google_search_protocol() as ._parse_search_response().
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Fix wrong content type of bingtestservice responses.
- Make googletestservice.py script executable.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad.
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-03-26 18:59:34 +0000
+++ lib/lp/services/config/schema-lazr.conf 2018-04-12 21:30:28 +0000
@@ -788,6 +788,7 @@
# datatype: string, a url to a host
site: http://www.google.com/cse
+[sitesearch]
# url_rewrite_exceptions is a list of launchpad.net domains that must
# not be rewritten.
# datatype: string of space separated domains
@@ -824,12 +825,6 @@
# datatype: string
custom_config_id:
-# url_rewrite_exceptions is a list of launchpad.net domains that must
-# not be rewritten.
-# datatype: string of space separated domains
-# Example: help.launchpad.net login.launchpad.net
-url_rewrite_exceptions: help.launchpad.net
-
[gpghandler]
# Should we allow uploading keys to the keyserver?
# datatype: boolean
=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py 2018-04-10 16:15:17 +0000
+++ lib/lp/services/sitesearch/__init__.py 2018-04-12 21:30:28 +0000
@@ -53,9 +53,9 @@
def url_rewrite_exceptions(self):
"""A list of launchpad.net URLs that must not be rewritten.
- Configured in config.google.url_rewrite_exceptions.
+ Configured in config.sitesearch.url_rewrite_exceptions.
"""
- return config.google.url_rewrite_exceptions.split()
+ return config.sitesearch.url_rewrite_exceptions.split()
@property
def url_rewrite_scheme(self):
@@ -221,7 +221,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
- page_matches = self._parse_google_search_protocol(response.content)
+ page_matches = self._parse_search_response(response.content)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -272,7 +272,7 @@
"""
return self._getElementsByAttributeValue(doc, path, name, value)[0]
- def _parse_google_search_protocol(self, gsp_xml):
+ def _parse_search_response(self, gsp_xml):
"""Return a `PageMatches` object.
:param gsp_xml: A string that should be Google Search Protocol
@@ -397,7 +397,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
- page_matches = self._parse_bing_response(response.content, start)
+ page_matches = self._parse_search_response(response.content, start)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -430,7 +430,7 @@
'Ocp-Apim-Subscription-Key': self.subscription_key,
}
- def _parse_bing_response(self, bing_json, start=0):
+ def _parse_search_response(self, bing_json, start=0):
"""Return a `PageMatches` object.
:param bing_json: A string containing Bing Custom Search API v7 JSON.
=== modified file 'lib/lp/services/sitesearch/bingtestservice.py'
--- lib/lp/services/sitesearch/bingtestservice.py 2018-03-28 21:28:12 +0000
+++ lib/lp/services/sitesearch/bingtestservice.py 2018-04-12 21:30:28 +0000
@@ -8,6 +8,10 @@
when given certain user-configurable URLs.
"""
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
import logging
import os
@@ -27,7 +31,7 @@
class BingRequestHandler(testservice.RequestHandler):
- default_content_type = 'text/xml; charset=UTF-8'
+ default_content_type = 'application/json; charset=UTF-8'
log = log
mapfile = config.bing_test_service.mapfile
content_dir = config.bing_test_service.canned_response_directory
=== modified file 'lib/lp/services/sitesearch/doc/bing-searchservice.txt'
--- lib/lp/services/sitesearch/doc/bing-searchservice.txt 2018-04-10 16:15:17 +0000
+++ lib/lp/services/sitesearch/doc/bing-searchservice.txt 2018-04-12 21:30:28 +0000
@@ -72,7 +72,7 @@
of returned results.
The first search for 'bugs' returned a subset of items in the
-ISearchResult. There are 25 total items, but the results contains the
+ISearchResult. There are 25 total items, but the results contain the
first 20 matches (because they start at index 0).
>>> first_page_matches.start
@@ -109,7 +109,7 @@
'http://launchpad.dev/mb'
'http://launchpad.dev/bugs'
-An empty PageMatches is returns if there are no results.
+An empty PageMatches is returned if there are no results.
>>> no_page_matches = bing_search.search(terms='fnord')
>>> no_page_matches.start
@@ -239,7 +239,7 @@
Bing Search response parsing
============================
-The BingSearchService's _parse_bing_response() expects a JSON response to
+The BingSearchService's _parse_search_response() expects a JSON response to
create the PageMatch and PageMatches objects. An error is raised when
the JSON document cannot be parsed into objects.
@@ -261,7 +261,7 @@
{...
"totalEstimatedMatches": "~25"...
- >>> bing_search._parse_bing_response(data)
+ >>> bing_search._parse_search_response(data)
Traceback (most recent call last):
...
SiteSearchResponseError: Could not get the total from the
@@ -278,7 +278,7 @@
{...
"totalEstimatedMatches": -25...
- >>> bing_search._parse_bing_response(data).total
+ >>> bing_search._parse_search_response(data).total
0
A PageMatch requires a title, url, and a summary. If those elements cannot
@@ -290,7 +290,7 @@
... base_path, 'bingsearchservice-missing-title.json')
>>> with open(json_file_name, 'r') as json_file:
... data = json_file.read()
- >>> page_matches = bing_search._parse_bing_response(data)
+ >>> page_matches = bing_search._parse_search_response(data)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -308,7 +308,7 @@
... base_path, 'bingsearchservice-missing-summary.json')
>>> with open(json_file_name, 'r') as json_file:
... data = json_file.read()
- >>> page_matches = bing_search._parse_bing_response(data)
+ >>> page_matches = bing_search._parse_search_response(data)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -324,7 +324,7 @@
... base_path, 'bingsearchservice-missing-url.json')
>>> with open(json_file_name, 'r') as json_file:
... data = json_file.read()
- >>> page_matches = bing_search._parse_bing_response(data)
+ >>> page_matches = bing_search._parse_search_response(data)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -344,7 +344,7 @@
... base_path, 'bingsearchservice-no-meaningful-results.json')
>>> with open(json_file_name, 'r') as json_file:
... data = json_file.read()
- >>> page_matches = bing_search._parse_bing_response(data)
+ >>> page_matches = bing_search._parse_search_response(data)
>>> len(page_matches)
0
@@ -353,7 +353,7 @@
>>> json_file_name = path.join(base_path, 'bingsearchservice-xss.json')
>>> with open(json_file_name, 'r') as json_file:
... data = json_file.read()
- >>> page_matches = bing_search._parse_bing_response(data)
+ >>> page_matches = bing_search._parse_search_response(data)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -370,7 +370,7 @@
-------------
The URL scheme used in the rewritten URL is configured in
-config.bing.url_rewrite_scheme. The hostname is set in the shared
+config.vhosts.use_https. The hostname is set in the shared
key config.vhost.mainsite.hostname.
>>> config.vhosts.use_https
@@ -404,10 +404,10 @@
'http://launchpad.dev/ubuntu'
There is a list of URLs that are not rewritten configured in
-config.bing.url_rewrite_exceptions. For example, help.launchpad.net
+config.sitesearch.url_rewrite_exceptions. For example, help.launchpad.net
is only run in one environment, so links to that site will be preserved.
- >>> config.bing.url_rewrite_exceptions
+ >>> config.sitesearch.url_rewrite_exceptions
'help.launchpad.net'
>>> page_match.url_rewrite_exceptions
['help.launchpad.net']
=== modified file 'lib/lp/services/sitesearch/doc/google-searchservice.txt'
--- lib/lp/services/sitesearch/doc/google-searchservice.txt 2018-03-27 20:41:35 +0000
+++ lib/lp/services/sitesearch/doc/google-searchservice.txt 2018-04-12 21:30:28 +0000
@@ -72,7 +72,7 @@
of returned results.
The first search for 'bugs' returned a subset of items in the
-ISearchResult. There are 25 total items, but the results contains the
+ISearchResult. There are 25 total items, but the results contain the
first 20 matches (because they start at index 0).
>>> first_page_matches.start
@@ -109,7 +109,7 @@
'http://bugs.launchpad.dev/bugs/bugtrackers'
'http://bugs.launchpad.dev/bugs/bugtrackers/debbugs'
-An empty PageMatches is returns if there are no results.
+An empty PageMatches is returned if there are no results.
>>> no_page_matches = google_search.search(terms='fnord')
>>> no_page_matches.start
@@ -248,7 +248,7 @@
Google Search Protocol parsing
==============================
-The GoogleSearchService's _parse_google_search_protocol() requires a
+The GoogleSearchService's _parse_search_response() requires a
subset of the GSP 3.2 markup to create the PageMatch and PageMatches
objects. An error is raised when the XML document cannot be parsed into
objects. The probable cause of an error is that Google returned a new
@@ -272,7 +272,7 @@
<...
<PARAM name="start" value="" original_value="0"/>...
- >>> google_search._parse_google_search_protocol(gsp_xml)
+ >>> google_search._parse_search_response(gsp_xml)
Traceback (most recent call last):
...
GoogleWrongGSPVersion: Could not get the 'start' from the
@@ -294,7 +294,7 @@
<RES SN="1" EN="1">
<M>~1</M>...
- >>> google_search._parse_google_search_protocol(gsp_xml)
+ >>> google_search._parse_search_response(gsp_xml)
Traceback (most recent call last):
...
GoogleWrongGSPVersion: Could not get the 'total' from the
@@ -313,7 +313,7 @@
<RES SN="1" EN="1">
<M>-1</M>...
- >>> google_search._parse_google_search_protocol(gsp_xml).total
+ >>> google_search._parse_search_response(gsp_xml).total
0
A PageMatch requires a title, url, and a summary. If those elements
@@ -350,7 +350,7 @@
<UE>https://bugs.launchpad.net/bugs/205991</UE>
<T>Bug #205991 in Ubuntu: ... pair Bluetooth Logitech ...</T>...
- >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+ >>> page_matches = google_search._parse_search_response(gsp_xml)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -394,7 +394,7 @@
<RK>0</RK>
<S>Discuss what needs to be done for <b>Gobuntu</b>...
- >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+ >>> page_matches = google_search._parse_search_response(gsp_xml)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -433,7 +433,7 @@
<UE>https://blueprints.launchpad.net/ubuntu/%2Bspec/gobuntu-hardy</UE>
<T>Blueprint: <b>Gobuntu</b> 8.04</T>...
- >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+ >>> page_matches = google_search._parse_search_response(gsp_xml)
>>> len(page_matches)
1
>>> page_matches[0].title
@@ -499,7 +499,7 @@
<HAS><L/><RT/></HAS>
</RESULT>...
- >>> google_search._parse_google_search_protocol(gsp_xml)
+ >>> google_search._parse_search_response(gsp_xml)
Traceback (most recent call last):
...
GoogleWrongGSPVersion: Could not get any PageMatches from the
@@ -528,7 +528,7 @@
<br>
tracker allows collaboration</S>...
- >>> page_matches = google_search._parse_google_search_protocol(gsp_xml)
+ >>> page_matches = google_search._parse_search_response(gsp_xml)
>>> page_match = page_matches[0]
>>> page_match.summary
u'<b>Bug</b> tracking <b>...</b> Search
@@ -542,7 +542,7 @@
-------------
The URL scheme used in the rewritten URL is configured in
-config.google.url_rewrite_scheme. The hostname is set in the shared
+config.vhosts.use_https. The hostname is set in the shared
key config.vhost.mainsite.hostname.
>>> config.vhosts.use_https
@@ -576,10 +576,10 @@
'http://launchpad.dev/ubuntu'
There is a list of URLs that are not rewritten configured in
-config.google.url_rewrite_exceptions. For example, help.launchpad.net
+config.sitesearch.url_rewrite_exceptions. For example, help.launchpad.net
is only run in one environment, so links to that site will be preserved.
- >>> config.google.url_rewrite_exceptions
+ >>> config.sitesearch.url_rewrite_exceptions
'help.launchpad.net'
>>> page_match.url_rewrite_exceptions
['help.launchpad.net']
=== modified file 'lib/lp/services/sitesearch/googletestservice.py' (properties changed: -x to +x)
--- lib/lp/services/sitesearch/googletestservice.py 2018-03-28 15:58:21 +0000
+++ lib/lp/services/sitesearch/googletestservice.py 2018-04-12 21:30:28 +0000
@@ -7,6 +7,9 @@
This script runs a simple HTTP server. The server returns XML files
when given certain user-configurable URLs.
"""
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
import logging
import os
=== modified file 'lib/lp/services/sitesearch/tests/test_bing.py'
--- lib/lp/services/sitesearch/tests/test_bing.py 2018-04-10 16:56:16 +0000
+++ lib/lp/services/sitesearch/tests/test_bing.py 2018-04-12 21:30:28 +0000
@@ -57,23 +57,23 @@
self.assertRaises(
SiteSearchResponseError, self.search_service.search, 'fnord')
- def test_parse_bing_response_TypeError(self):
+ def test_parse_search_response_TypeError(self):
# The method converts TypeError to SiteSearchResponseError.
self.assertRaises(
SiteSearchResponseError,
- self.search_service._parse_bing_response, None)
+ self.search_service._parse_search_response, None)
- def test_parse_bing_response_ValueError(self):
+ def test_parse_search_response_ValueError(self):
# The method converts ValueError to SiteSearchResponseError.
self.assertRaises(
SiteSearchResponseError,
- self.search_service._parse_bing_response, '')
+ self.search_service._parse_search_response, '')
- def test_parse_bing_response_KeyError(self):
+ def test_parse_search_response_KeyError(self):
# The method converts KeyError to SiteSearchResponseError.
self.assertRaises(
SiteSearchResponseError,
- self.search_service._parse_bing_response, '{}')
+ self.search_service._parse_search_response, '{}')
class FunctionalTestBingSearchService(TestCase):
=== modified file 'lib/lp/services/sitesearch/tests/test_google.py'
--- lib/lp/services/sitesearch/tests/test_google.py 2018-03-28 19:31:02 +0000
+++ lib/lp/services/sitesearch/tests/test_google.py 2018-04-12 21:30:28 +0000
@@ -52,16 +52,16 @@
self.assertRaises(
SiteSearchResponseError, self.search_service.search, 'fnord')
- def test___parse_google_search_protocol_SyntaxError(self):
+ def test_parse_search_response_SyntaxError(self):
# The method converts SyntaxError to SiteSearchResponseError.
self.useFixture(MockPatch(
'lp.services.sitesearch.urlfetch',
side_effect=SyntaxError('oops')))
self.assertRaises(
SiteSearchResponseError,
- self.search_service._parse_google_search_protocol, '')
+ self.search_service._parse_search_response, '')
- def test___parse_google_search_protocol_IndexError(self):
+ def test_parse_search_response_IndexError(self):
# The method converts IndexError to SiteSearchResponseError.
self.useFixture(MockPatch(
'lp.services.sitesearch.urlfetch', side_effect=IndexError('oops')))
@@ -70,4 +70,4 @@
'<GSP VER="3.2"></GSP>')
self.assertRaises(
SiteSearchResponseError,
- self.search_service._parse_google_search_protocol, data)
+ self.search_service._parse_search_response, data)
=== modified file 'lib/lp/services/sitesearch/testservice.py' (properties changed: +x to -x)
Follow ups
