diff --git a/cps/metadata_provider/google.py b/cps/metadata_provider/google.py index d53d2e77e..78aa56cae 100644 --- a/cps/metadata_provider/google.py +++ b/cps/metadata_provider/google.py @@ -22,6 +22,8 @@ from urllib.parse import quote from datetime import datetime import requests +from requests.adapters import HTTPAdapter +from urllib3.util.retry import Retry from cps import logger, config from cps.isoLanguages import get_lang3, get_language_name @@ -29,6 +31,13 @@ from cps.services.Metadata import MetaRecord, MetaSourceInfo, Metadata log = logger.create() +# Google answers a share of requests with a transient 5xx, which is otherwise +# indistinguishable from "not found". raise_on_status lets the last response +# through so it is logged with its status instead of as a MaxRetryError. +RETRY = Retry(total=3, backoff_factor=0.3, status_forcelist=(500, 502, 503, 504), + raise_on_status=False) +TIMEOUT = (5, 15) + class Google(Metadata): __name__ = "Google Books" @@ -38,7 +47,6 @@ class Google(Metadata): BOOK_URL = "https://books.google.com/books?id=" SEARCH_URL = "https://www.googleapis.com/books/v1/volumes?q=" ISBN_TYPE = "ISBN_13" - API_KEY = "&key=" + config.config_googlebooks_api_key def search( self, query: str, generic_cover: str = "", locale: str = "en" @@ -50,12 +58,21 @@ class Google(Metadata): if title_tokens: tokens = [quote(t.encode("utf-8")) for t in title_tokens] query = "+".join(tokens) + # Read per search, so a key saved in the admin page takes effect + # without a restart. + key = config.config_googlebooks_api_key try: - results = requests.get(Google.SEARCH_URL + query + Google.API_KEY) + # A session per call: Session is not thread-safe and the + # providers are searched in a thread pool. + session = requests.Session() + session.mount("https://", HTTPAdapter(max_retries=RETRY)) + results = session.get( + Google.SEARCH_URL + query + ("&key=" + key if key else ""), + timeout=TIMEOUT) results.raise_for_status() except Exception as e: log.warning(e) - return [] + return None # None is a failed search, [] is an empty one for result in results.json().get("items", []): val.append( self._parse_search_result( diff --git a/cps/search_metadata.py b/cps/search_metadata.py index d8a12983d..5646cf88d 100644 --- a/cps/search_metadata.py +++ b/cps/search_metadata.py @@ -157,6 +157,7 @@ def metadata_search(): data = list() active = current_user.view_settings.get("metadata", {}) locale = get_locale() + asked = failed = 0 if query: static_cover = url_for("static", filename="generic_cover.jpg") # ret = cl[0].search(query, static_cover, locale) @@ -166,6 +167,20 @@ def metadata_search(): for c in cl if active.get(c.__id__, True) } + asked = len(meta) for future in concurrent.futures.as_completed(meta): - data.extend(_serialize_metadata_records(future.result())) + try: + records = future.result() + except Exception as e: + # One provider raising used to fail the whole request. + log.warning("Metadata provider %s failed: %s", meta[future].__id__, e) + records = None + if records is None: # failed, as opposed to found nothing + failed += 1 + continue + data.extend(_serialize_metadata_records(records)) + # Nothing to say about the keyword if every source failed, so 502 makes the + # client show "Search error!" rather than "No Result(s) found!". + if asked and failed == asked: + return make_response(jsonify(data), 502) return make_response(jsonify(data))