← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad

 

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad.

Commit message:
A few extra renamed classes from Google* to SiteSearch*.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/rename-google-as-sitesearch-extra/+merge/342148

A few extra renamed classes from Google* to SiteSearch*.

- Rename GoogleBatchNavigator as SiteSearchBatchNavigator.
- Rename GoogleResponseError as SiteSearchResponseError.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad.
=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py	2018-03-16 14:50:01 +0000
+++ lib/lp/app/browser/root.py	2018-03-26 21:14:59 +0000
@@ -42,7 +42,7 @@
 from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.propertycache import cachedproperty
 from lp.services.sitesearch.interfaces import (
-    GoogleResponseError,
+    SiteSearchResponseError,
     ISearchService,
     )
 from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet
@@ -520,14 +520,14 @@
         try:
             page_matches = google_search.search(
                 terms=query_terms, start=start)
-        except GoogleResponseError:
+        except SiteSearchResponseError:
             # There was a connectivity or Google service issue that means
             # there is no data available at this moment.
             self.has_page_service = False
             return None
         if len(page_matches) == 0:
             return None
-        navigator = GoogleBatchNavigator(
+        navigator = SiteSearchBatchNavigator(
             page_matches, self.request, start=start)
         navigator.setHeadings(*self.batch_heading)
         return navigator
@@ -589,7 +589,7 @@
         return self.start + len(self.list._window)
 
 
-class GoogleBatchNavigator(BatchNavigator):
+class SiteSearchBatchNavigator(BatchNavigator):
     """A batch navigator with a fixed size of 20 items per batch."""
 
     _batch_factory = WindowedListBatch
@@ -614,7 +614,7 @@
         :param callback: Not used.
         """
         results = WindowedList(results, start, results.total)
-        super(GoogleBatchNavigator, self).__init__(results, request,
+        super(SiteSearchBatchNavigator, self).__init__(results, request,
             start=start, size=size, callback=callback,
             transient_parameters=transient_parameters,
             force_start=force_start, range_factory=range_factory)

=== modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt'
--- lib/lp/app/browser/tests/launchpad-search-pages.txt	2018-03-16 14:02:16 +0000
+++ lib/lp/app/browser/tests/launchpad-search-pages.txt	2018-03-26 21:14:59 +0000
@@ -416,7 +416,7 @@
     >>> search_view.has_matches
     True
     >>> search_view.pages
-    <...GoogleBatchNavigator ...>
+    <...SiteSearchBatchNavigator ...>
 
 The GoogleSearchService may not be available due to connectivity problems.
 The view's has_page_service attribute reports when the search was performed
@@ -444,7 +444,7 @@
     >>> search_view.batch_heading
     (u'other page matching "launchpad"', u'other pages matching "launchpad"')
 
-The GoogleBatchNavigator behaves like most BatchNavigators, except that
+The SiteSearchBatchNavigator behaves like most BatchNavigators, except that
 its batch size is always 20. The size restriction conforms to Google's
 maximum number of results that can be returned per request.
 
@@ -625,7 +625,7 @@
       </div>
     </form>
 
-WindowedList and GoogleBatchNavigator
+WindowedList and SiteSearchBatchNavigator
 -------------------------------------
 
 The LaunchpadSearchView uses two helper classes to work with
@@ -635,7 +635,7 @@
 or fewer PageMatches of what could be thousands of matches. Google
 requires client's to make repeats request to step though the batches of
 matches. The Windowed list is a list that contains only a subset of its
-reported size. It is used to make batches in the GoogleBatchNavigator.
+reported size. It is used to make batches in the SiteSearchBatchNavigator.
 
 For example, the last batch of the 'bug' search contained 5 of the 25
 matching pages. The WindowList claims to be 25 items in length, but
@@ -657,14 +657,14 @@
     >>> results[18, 22]
     [None, None, <...PageMatch ...>, <...PageMatch ...>]
 
-The GoogleBatchNavigator restricts the batch size to 20. the 'batch'
+The SiteSearchBatchNavigator restricts the batch size to 20. the 'batch'
 parameter that comes from the URL is ignored. For example, setting
 the 'batch' parameter to 100 has no affect upon the Google search
 or on the navigator object.
 
-    >>> from lp.app.browser.root import GoogleBatchNavigator
+    >>> from lp.app.browser.root import SiteSearchBatchNavigator
 
-    >>> GoogleBatchNavigator.batch_variable_name
+    >>> SiteSearchBatchNavigator.batch_variable_name
     'batch'
 
     >>> search_view = getSearchView(
@@ -687,7 +687,7 @@
     >>> page_matches._matches = matches
     >>> page_matches.start = 0
     >>> page_matches.total = 100
-    >>> navigator = GoogleBatchNavigator(
+    >>> navigator = SiteSearchBatchNavigator(
     ...     page_matches, search_view.request, page_matches.start, size=100)
     >>> navigator.currentBatch().size
     20
@@ -705,7 +705,7 @@
 
     >>> matches = list(range(0, 3))
     >>> page_matches._matches = matches
-    >>> navigator = GoogleBatchNavigator(
+    >>> navigator = SiteSearchBatchNavigator(
     ...     page_matches, search_view.request, page_matches.start, size=100)
     >>> batch = navigator.currentBatch()
     >>> batch.size

=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/__init__.py	2018-03-26 21:14:59 +0000
@@ -25,7 +25,7 @@
 
 from lp.services.config import config
 from lp.services.sitesearch.interfaces import (
-    GoogleResponseError,
+    SiteSearchResponseError,
     GoogleWrongGSPVersion,
     ISearchResult,
     ISearchResults,
@@ -214,7 +214,7 @@
         except (TimeoutError, requests.RequestException) as error:
             # Google search service errors are not code errors. Let the
             # call site choose to handle the unavailable service.
-            raise GoogleResponseError(
+            raise SiteSearchResponseError(
                 "The response errored: %s" % str(error))
         finally:
             action.finish()
@@ -281,7 +281,7 @@
             version 3.2 XML. There is no guarantee that other GSP versions
             can be parsed.
         :return: `ISearchResults` (PageMatches).
-        :raise: `GoogleResponseError` if the xml is incomplete.
+        :raise: `SiteSearchResponseError` if the xml is incomplete.
         :raise: `GoogleWrongGSPVersion` if the xml cannot be parsed.
         """
         try:
@@ -289,7 +289,8 @@
             start_param = self._getElementByAttributeValue(
                 gsp_doc, './PARAM', 'name', 'start')
         except (SyntaxError, IndexError):
-            raise GoogleResponseError("The response was incomplete, no xml.")
+            raise SiteSearchResponseError(
+                "The response was incomplete, no xml.")
         try:
             start = int(start_param.get('value'))
         except (AttributeError, ValueError):

=== modified file 'lib/lp/services/sitesearch/doc/google-searchservice.txt'
--- lib/lp/services/sitesearch/doc/google-searchservice.txt	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/doc/google-searchservice.txt	2018-03-26 21:14:59 +0000
@@ -617,7 +617,7 @@
     >>> google_search.search(terms='bug')
     Traceback (most recent call last):
      ...
-    GoogleResponseError: ... timeout exceeded.
+    SiteSearchResponseError: ... timeout exceeded.
 
     # Restore the configuration and the timeout state.
     >>> timeout_data = config.pop('timeout_data')

=== modified file 'lib/lp/services/sitesearch/interfaces.py'
--- lib/lp/services/sitesearch/interfaces.py	2013-01-07 02:40:55 +0000
+++ lib/lp/services/sitesearch/interfaces.py	2018-03-26 21:14:59 +0000
@@ -9,8 +9,8 @@
     'ISearchResult',
     'ISearchResults',
     'ISearchService',
-    'GoogleResponseError',
     'GoogleWrongGSPVersion',
+    'SiteSearchResponseError',
     ]
 
 from zope.interface import Interface
@@ -74,8 +74,8 @@
     """Raised when the content is not parsable Google Search Protocol XML."""
 
 
-class GoogleResponseError(SyntaxError):
-    """Raised when Google's response is not contain valid XML."""
+class SiteSearchResponseError(ValueError):
+    """Raised when the search engine's response cannot be parsed."""
 
 
 class ISearchService(Interface):

=== modified file 'lib/lp/services/sitesearch/tests/test_google.py'
--- lib/lp/services/sitesearch/tests/test_google.py	2018-03-16 14:02:16 +0000
+++ lib/lp/services/sitesearch/tests/test_google.py	2018-03-26 21:14:59 +0000
@@ -13,7 +13,7 @@
     )
 
 from lp.services.sitesearch import GoogleSearchService
-from lp.services.sitesearch.interfaces import GoogleResponseError
+from lp.services.sitesearch.interfaces import SiteSearchResponseError
 from lp.services.timeout import TimeoutError
 from lp.testing import TestCase
 from lp.testing.layers import LaunchpadFunctionalLayer
@@ -49,37 +49,37 @@
         self.search_service = GoogleSearchService()
 
     def test_search_converts_HTTPError(self):
-        # The method converts HTTPError to GoogleResponseError.
+        # The method converts HTTPError to SiteSearchResponseError.
         args = ('url', 500, 'oops', {}, None)
         with urlfetch_exception(HTTPError, *args):
             self.assertRaises(
-                GoogleResponseError, self.search_service.search, 'fnord')
+                SiteSearchResponseError, self.search_service.search, 'fnord')
 
     def test_search_converts_ConnectionError(self):
-        # The method converts ConnectionError to GoogleResponseError.
+        # The method converts ConnectionError to SiteSearchResponseError.
         with urlfetch_exception(ConnectionError, 'oops'):
             self.assertRaises(
-                GoogleResponseError, self.search_service.search, 'fnord')
+                SiteSearchResponseError, self.search_service.search, 'fnord')
 
     def test_search_converts_TimeoutError(self):
-        # The method converts TimeoutError to GoogleResponseError.
+        # The method converts TimeoutError to SiteSearchResponseError.
         with urlfetch_exception(TimeoutError, 'oops'):
             self.assertRaises(
-                GoogleResponseError, self.search_service.search, 'fnord')
+                SiteSearchResponseError, self.search_service.search, 'fnord')
 
     def test___parse_google_search_protocol_SyntaxError(self):
-        # The method converts SyntaxError to GoogleResponseError.
+        # The method converts SyntaxError to SiteSearchResponseError.
         with urlfetch_exception(SyntaxError, 'oops'):
             self.assertRaises(
-                GoogleResponseError,
+                SiteSearchResponseError,
                 self.search_service._parse_google_search_protocol, '')
 
     def test___parse_google_search_protocol_IndexError(self):
-        # The method converts IndexError to GoogleResponseError.
+        # 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(
-                GoogleResponseError,
+                SiteSearchResponseError,
                 self.search_service._parse_google_search_protocol, data)


Follow ups