launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22322
[Merge] lp:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad.
Commit message:
Generalize googletestservice.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/generalized-sitesearch-testservice/+merge/342312
Generalize sitesearch.googletestservice, making it a thin wrapper around new sitesearch.testservice, while keeping the patch as short as possible. Also, assorted improvements in tests.
This will help in reducing code duplication when the Bing search engine is introduced.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad.
=== modified file 'lib/lp/app/stories/launchpad-root/site-search.txt'
--- lib/lp/app/stories/launchpad-root/site-search.txt 2016-01-26 15:47:37 +0000
+++ lib/lp/app/stories/launchpad-root/site-search.txt 2018-03-28 17:01:31 +0000
@@ -1,9 +1,9 @@
Site-wide Search
================
-Launchpad features a site-wide search that combines Google's site-
-specific search with Launchpad's prominent objects (projects, bugs,
-teams, etc.).
+Launchpad features a site-wide search that combines an external search
+engine's site-specific search with Launchpad's prominent objects (projects,
+bugs, teams, etc.).
# Our very helpful function for printing all the page results.
@@ -81,9 +81,9 @@
3: Firefox is slow and consumes too much RAM
...
-An arbitrary search returns a list of Google search results. The user
-searches for "bug" and sees a listing of matching pages. The navigation
-states that the page is showing 1 through 20 of 25 total results.
+An arbitrary search returns a list of the search engine's search results.
+The user searches for "bug" and sees a listing of matching pages. The
+navigation states that the page is showing 1 through 20 of 25 total results.
# Use our pre-defined search results for the 'bug' search.
@@ -248,9 +248,10 @@
Searches when there is no page service
--------------------------------------
-Google may not be available when the search is performed. This is often
-caused by temporary connectivity problems. A message is displayed that
-explains that the search can be performed again to find matching pages.
+The search provider may not be available when the search is performed.
+This is often caused by temporary connectivity problems. A message is
+displayed that explains that the search can be performed again to find
+matching pages.
>>> search_for('gnomebaker')
>>> print find_tag_by_id(anon_browser.contents, 'no-page-service')
=== added file 'lib/lp/services/sitesearch/googletestservice.py'
--- lib/lp/services/sitesearch/googletestservice.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/sitesearch/googletestservice.py 2018-03-28 17:01:31 +0000
@@ -0,0 +1,79 @@
+#!/usr/bin/python
+#
+# Copyright 2018 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""
+This script runs a simple HTTP server. The server returns XML files
+when given certain user-configurable URLs.
+"""
+
+import logging
+import os
+
+from six.moves.BaseHTTPServer import HTTPServer
+
+from lp.services.config import config
+from lp.services.osutils import ensure_directory_exists
+from lp.services.pidfile import make_pidfile
+from lp.services.sitesearch import testservice
+
+
+# Set up basic logging.
+log = logging.getLogger(__name__)
+
+# The default service name, used by the Launchpad service framework.
+service_name = 'google-webservice'
+
+
+class GoogleRequestHandler(testservice.RequestHandler):
+ default_content_type = 'text/xml; charset=UTF-8'
+ log = log
+ mapfile = config.google_test_service.mapfile
+ content_dir = config.google_test_service.canned_response_directory
+
+
+def start_as_process():
+ return testservice.start_as_process('googletestservice')
+
+
+def get_service_endpoint():
+ """Return the host and port that the service is running on."""
+ return testservice.hostpair(config.google.site)
+
+
+def service_is_available():
+ host, port = get_service_endpoint()
+ return testservice.service_is_available(host, port)
+
+
+def wait_for_service():
+ host, port = get_service_endpoint()
+ return testservice.wait_for_service(host, port)
+
+
+def kill_running_process():
+ global service_name
+ host, port = get_service_endpoint()
+ return testservice.kill_running_process(service_name, host, port)
+
+
+def main():
+ """Run the HTTP server."""
+ # Redirect our service output to a log file.
+ global log
+ ensure_directory_exists(os.path.dirname(config.google_test_service.log))
+ filelog = logging.FileHandler(config.google_test_service.log)
+ log.addHandler(filelog)
+ log.setLevel(logging.DEBUG)
+
+ # To support service shutdown we need to create a PID file that is
+ # understood by the Launchpad services framework.
+ global service_name
+ make_pidfile(service_name)
+
+ host, port = get_service_endpoint()
+ server = HTTPServer((host, port), GoogleRequestHandler)
+
+ log.info("Starting HTTP Google webservice server on port %s", port)
+ server.serve_forever()
=== modified file 'lib/lp/services/sitesearch/tests/test_google.py'
--- lib/lp/services/sitesearch/tests/test_google.py 2018-03-26 21:06:51 +0000
+++ lib/lp/services/sitesearch/tests/test_google.py 2018-03-28 17:01:31 +0000
@@ -5,8 +5,8 @@
__metaclass__ = type
-from contextlib import contextmanager
+from fixtures import MockPatch
from requests.exceptions import (
ConnectionError,
HTTPError,
@@ -19,26 +19,6 @@
from lp.testing.layers import LaunchpadFunctionalLayer
-@contextmanager
-def urlfetch_exception(test_error, *args):
- """Raise an error during the execution of urlfetch.
-
- This function replaces urlfetch() with a function that
- raises an error.
- """
-
- def raise_exception(url):
- raise test_error(*args)
-
- from lp.services import timeout
- original_urlfetch = timeout.urlfetch
- timeout.urlfetch = raise_exception
- try:
- yield
- finally:
- timeout.urlfetch = original_urlfetch
-
-
class TestGoogleSearchService(TestCase):
"""Test GoogleSearchService."""
@@ -51,35 +31,42 @@
def test_search_converts_HTTPError(self):
# The method converts HTTPError to SiteSearchResponseError.
args = ('url', 500, 'oops', {}, None)
- with urlfetch_exception(HTTPError, *args):
- self.assertRaises(
- SiteSearchResponseError, self.search_service.search, 'fnord')
+ self.useFixture(MockPatch(
+ 'lp.services.sitesearch.urlfetch', side_effect=HTTPError(*args)))
+ self.assertRaises(
+ SiteSearchResponseError, self.search_service.search, 'fnord')
def test_search_converts_ConnectionError(self):
# The method converts ConnectionError to SiteSearchResponseError.
- with urlfetch_exception(ConnectionError, 'oops'):
- self.assertRaises(
- SiteSearchResponseError, self.search_service.search, 'fnord')
+ self.useFixture(MockPatch(
+ 'lp.services.sitesearch.urlfetch',
+ side_effect=ConnectionError('oops')))
+ self.assertRaises(
+ SiteSearchResponseError, self.search_service.search, 'fnord')
def test_search_converts_TimeoutError(self):
# The method converts TimeoutError to SiteSearchResponseError.
- with urlfetch_exception(TimeoutError, 'oops'):
- self.assertRaises(
- SiteSearchResponseError, self.search_service.search, 'fnord')
+ self.useFixture(MockPatch(
+ 'lp.services.sitesearch.urlfetch',
+ side_effect=TimeoutError('oops')))
+ self.assertRaises(
+ SiteSearchResponseError, self.search_service.search, 'fnord')
def test___parse_google_search_protocol_SyntaxError(self):
# The method converts SyntaxError to SiteSearchResponseError.
- with urlfetch_exception(SyntaxError, 'oops'):
- self.assertRaises(
- SiteSearchResponseError,
- self.search_service._parse_google_search_protocol, '')
+ self.useFixture(MockPatch(
+ 'lp.services.sitesearch.urlfetch', side_effect=SyntaxError('oops')))
+ self.assertRaises(
+ SiteSearchResponseError,
+ self.search_service._parse_google_search_protocol, '')
def test___parse_google_search_protocol_IndexError(self):
# The method converts IndexError to SiteSearchResponseError.
- with urlfetch_exception(IndexError, 'oops'):
- data = (
- '<?xml version="1.0" encoding="UTF-8" standalone="no"?>'
- '<GSP VER="3.2"></GSP>')
- self.assertRaises(
- SiteSearchResponseError,
- self.search_service._parse_google_search_protocol, data)
+ self.useFixture(MockPatch(
+ 'lp.services.sitesearch.urlfetch', side_effect=IndexError('oops')))
+ data = (
+ '<?xml version="1.0" encoding="UTF-8" standalone="no"?>'
+ '<GSP VER="3.2"></GSP>')
+ self.assertRaises(
+ SiteSearchResponseError,
+ self.search_service._parse_google_search_protocol, data)
=== renamed file 'lib/lp/services/sitesearch/googletestservice.py' => 'lib/lp/services/sitesearch/testservice.py'
--- lib/lp/services/sitesearch/googletestservice.py 2018-03-26 21:22:03 +0000
+++ lib/lp/services/sitesearch/testservice.py 2018-03-28 17:01:31 +0000
@@ -1,54 +1,37 @@
#!/usr/bin/python
#
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""
-This script runs a simple HTTP server. The server returns XML files
+This script runs a simple HTTP server. The server returns files
when given certain user-configurable URLs.
"""
import errno
-import logging
import os
import signal
import socket
import subprocess
import time
-from six.moves.BaseHTTPServer import (
- BaseHTTPRequestHandler,
- HTTPServer,
- )
+from six.moves.BaseHTTPServer import BaseHTTPRequestHandler
-from lp.services.config import config
-from lp.services.osutils import (
- ensure_directory_exists,
- remove_if_exists,
- )
+from lp.services.osutils import remove_if_exists
from lp.services.pidfile import (
get_pid,
- make_pidfile,
pidfile_path,
)
from lp.services.webapp.url import urlsplit
-# Set up basic logging.
-log = logging.getLogger(__name__)
-
-# The default service name, used by the Launchpad service framework.
-service_name = 'google-webservice'
-
-
-class GoogleRequestHandler(BaseHTTPRequestHandler):
- """Return an XML file depending on the requested URL."""
-
- default_content_type = 'text/xml; charset=UTF-8'
+
+class RequestHandler(BaseHTTPRequestHandler):
+ """Return a file depending on the requested URL."""
def do_GET(self):
"""See BaseHTTPRequestHandler in the Python Standard Library."""
- urlmap = url_to_xml_map()
+ urlmap = url_to_file_map(self.mapfile)
if self.path in urlmap:
self.return_file(urlmap[self.path])
else:
@@ -65,8 +48,7 @@
self.send_header('Content-Type', self.default_content_type)
self.end_headers()
- content_dir = config.google_test_service.canned_response_directory
- filepath = os.path.join(content_dir, filename)
+ filepath = os.path.join(self.content_dir, filename)
content_body = file(filepath).read()
self.wfile.write(content_body)
@@ -78,12 +60,11 @@
(self.address_string(),
self.log_date_time_string(),
format % args))
- log.info(message)
-
-
-def url_to_xml_map():
- """Return our URL-to-XML mapping as a dictionary."""
- mapfile = config.google_test_service.mapfile
+ self.log.info(message)
+
+
+def url_to_file_map(mapfile):
+ """Return our URL-to-file mapping as a dictionary."""
mapping = {}
for line in file(mapfile):
if line.startswith('#') or len(line.strip()) == 0:
@@ -95,12 +76,7 @@
return mapping
-def get_service_endpoint():
- """Return the host and port that the service is running on."""
- return hostpair(config.google.site)
-
-
-def service_is_available(timeout=2.0):
+def service_is_available(host, port, timeout=2.0):
"""Return True if the service is up and running.
:param timeout: BLOCK execution for at most 'timeout' seconds
@@ -108,7 +84,6 @@
"""
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.settimeout(timeout) # Block for 'timeout' seconds.
- host, port = get_service_endpoint()
try:
try:
sock.connect((host, port))
@@ -120,14 +95,13 @@
sock.close() # Clean up.
-def wait_for_service(timeout=15.0):
+def wait_for_service(host, port, timeout=15.0):
"""Poll the service and BLOCK until we can connect to it.
:param timeout: The socket should timeout after this many seconds.
Refer to the socket module documentation in the Standard Library
for possible timeout values.
"""
- host, port = get_service_endpoint()
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.settimeout(timeout) # Block for at most X seconds.
@@ -150,7 +124,7 @@
sock.close() # Clean up.
-def wait_for_service_shutdown(seconds_to_wait=10.0):
+def wait_for_service_shutdown(host, port, seconds_to_wait=10.0):
"""Poll the service until it shuts down.
Raises a RuntimeError if the service doesn't shut down within the allotted
@@ -160,8 +134,6 @@
:param seconds_to_wait: The number of seconds to wait for the socket to
open up.
"""
- host, port = get_service_endpoint()
-
start = time.time() # Record when we started polling.
try:
while True:
@@ -194,7 +166,7 @@
return (host, port)
-def start_as_process():
+def start_as_process(service_binary_name):
"""Run this file as a stand-alone Python script.
Returns a subprocess.Popen object. (See the `subprocess` module in
@@ -203,7 +175,7 @@
script = os.path.join(
os.path.dirname(__file__),
os.pardir, os.pardir, os.pardir, os.pardir, 'bin',
- 'googletestservice')
+ service_binary_name)
# Make sure we aren't using the parent stdin and stdout to avoid spam
# and have fewer things that can go wrong shutting down the process.
proc = subprocess.Popen(
@@ -213,9 +185,8 @@
return proc
-def kill_running_process():
+def kill_running_process(service_name, host, port):
"""Find and kill any running web service processes."""
- global service_name
try:
pid = get_pid(service_name)
except IOError:
@@ -233,7 +204,7 @@
# becomes available. Failing to do so causes a race condition
# between freeing the socket in the killed process, and
# opening it in the current one.
- wait_for_service_shutdown()
+ wait_for_service_shutdown(host, port)
except os.error as err:
if err.errno == errno.ESRCH:
# Whoops, we got a 'No such process' error. The PID file
@@ -243,24 +214,3 @@
remove_if_exists(pidfile_path(service_name))
else:
raise
-
-
-def main():
- """Run the HTTP server."""
- # Redirect our service output to a log file.
- global log
- ensure_directory_exists(os.path.dirname(config.google_test_service.log))
- filelog = logging.FileHandler(config.google_test_service.log)
- log.addHandler(filelog)
- log.setLevel(logging.DEBUG)
-
- # To support service shutdown we need to create a PID file that is
- # understood by the Launchpad services framework.
- global service_name
- make_pidfile(service_name)
-
- host, port = get_service_endpoint()
- server = HTTPServer((host, port), GoogleRequestHandler)
-
- log.info("Starting HTTP Google webservice server on port %s", port)
- server.serve_forever()
Follow ups
