Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query updates #84

Merged
merged 11 commits into from
Mar 7, 2024
36 changes: 34 additions & 2 deletions bookops_worldcat/_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
and others.
"""

from typing import Optional, Tuple, Union
from typing import Optional, Tuple, Union, List

import requests
from urllib3.util import Retry

from . import __title__, __version__
from .authorize import WorldcatAccessToken
Expand All @@ -24,6 +25,10 @@ def __init__(
5,
5,
),
total_retries: int = 0,
backoff_factor: float = 0,
status_forcelist: Optional[List[int]] = [],
klinga marked this conversation as resolved.
Show resolved Hide resolved
allowed_methods: Optional[List[str]] = None,
) -> None:
"""
Args:
Expand All @@ -32,9 +37,26 @@ def __init__(
request in the session
timeout: how long to wait for server to send data
before giving up
total_retries: optional number of times to retry a request that
failed or timed out. if total_retries argument is
not passed, any arguments passed to
backoff_factor, status_forcelist, and
allowed_methods will be ignored. default is 0
backoff_factor: if total_retries is not 0, the backoff
factor as a float to use to calculate amount of
time session will sleep before attempting request
again. default is 0
status_forcelist: if total_retries is not 0, a list of HTTP
status codes to automatically retry requests on.
if not specified, all failed requests will be
retried up to number of total_retries.
example: [500, 502, 503, 504]
allowed_methods: if total_retries is not 0, set of HTTP methods that
requests should be retried on. if not specified,
requests using any HTTP method verbs will be
retried. example: ["GET", "POST"]
"""
super().__init__()

self.authorization = authorization

if not isinstance(self.authorization, WorldcatAccessToken):
Expand All @@ -51,6 +73,16 @@ def __init__(

self.timeout = timeout

# if user provides retry args, create Retry object and mount adapter to session
if total_retries != 0:
retries = Retry(
total=total_retries,
backoff_factor=backoff_factor,
status_forcelist=status_forcelist,
allowed_methods=allowed_methods,
)
self.mount("https://", requests.adapters.HTTPAdapter(max_retries=retries))

self._update_authorization()

def _get_new_access_token(self) -> None:
Expand Down
36 changes: 33 additions & 3 deletions bookops_worldcat/metadata_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ def __init__(
authorization: WorldcatAccessToken,
agent: Optional[str] = None,
timeout: Union[int, float, Tuple[int, int], Tuple[float, float], None] = None,
total_retries: int = 0,
backoff_factor: float = 0,
status_forcelist: Optional[List[int]] = [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does mypy agree?

allowed_methods: Optional[List[str]] = None,
) -> None:
"""
Args:
Expand All @@ -32,8 +36,34 @@ def __init__(
header; usage strongly encouraged
timeout: how long to wait for server to send data before
giving up; default value is 5 seconds
"""
super().__init__(authorization, agent=agent, timeout=timeout)
total_retries: optional number of times to retry a request that
failed or timed out. if total_retries argument is
not passed, any arguments passed to
backoff_factor, status_forcelist, and
allowed_methods will be ignored. default is 0
backoff_factor: if total_retries is not 0, the backoff
factor as a float to use to calculate amount of
time session will sleep before attempting request
again. default is 0
status_forcelist: if total_retries is not 0, a list of HTTP
status codes to automatically retry requests on.
if not specified, all failed requests will be
retried up to number of total_retries.
example: [500, 502, 503, 504]
allowed_methods: if total_retries is not 0, set of HTTP methods that
requests should be retried on. if not specified,
requests using any HTTP method verbs will be
retried. example: ["GET", "POST"]
"""
super().__init__(
authorization,
agent=agent,
timeout=timeout,
total_retries=total_retries,
backoff_factor=backoff_factor,
status_forcelist=status_forcelist,
allowed_methods=allowed_methods,
)

def _split_into_legal_volume(
self, oclc_numbers: List[str] = [], n: int = 50
Expand Down Expand Up @@ -207,7 +237,7 @@ def get_institution_holdings(
self,
oclcNumbers: Union[str, List[Union[str, int]]],
hooks: Optional[Dict[str, Callable]] = None,
) -> List[Optional[Response]]:
) -> List[Response]:
"""
Retrieves Worlcat holdings status of a record with provided OCLC number.
The service automatically recognizes institution based on the issued access
Expand Down
14 changes: 6 additions & 8 deletions bookops_worldcat/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
import sys

from requests.models import PreparedRequest
from requests.exceptions import ConnectionError, HTTPError, Timeout

from requests.exceptions import ConnectionError, HTTPError, Timeout, RetryError
from .errors import WorldcatRequestError


Expand All @@ -19,12 +18,12 @@

class Query:
"""
Sends a request to OClC service and unifies received exceptions
Sends a request to OCLC service and unifies exceptions.
Query object handles refreshing expired token before request is
made to the web service.

`Query.response` attribute is `requests.Response` instance that
can be parsed to exctract received information from the web service.
can be parsed to extract information received from the web service.
"""

def __init__(
Expand Down Expand Up @@ -53,8 +52,6 @@ def __init__(
if session.authorization.is_expired():
session._get_new_access_token()

self.response = None
charlottekostelic marked this conversation as resolved.
Show resolved Hide resolved

try:
self.response = session.send(prepared_request, timeout=timeout)
self.response.raise_for_status()
Expand All @@ -64,7 +61,8 @@ def __init__(
f"{exc}. Server response: " # type: ignore
f"{self.response.content.decode('utf-8')}"
)
except (Timeout, ConnectionError):
except (Timeout, ConnectionError, RetryError):
raise WorldcatRequestError(f"Connection Error: {sys.exc_info()[0]}")
except:

except Exception:
raise WorldcatRequestError(f"Unexpected request error: {sys.exc_info()[0]}")
24 changes: 24 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ def __init__(self, *args, **kwargs):
raise requests.exceptions.ConnectionError


class MockRetryError:
def __init__(self, *args, **kwargs):
raise requests.exceptions.RetryError


class MockHTTPSessionResponse(Response):
def __init__(self, http_code):
self.status_code = http_code
Expand Down Expand Up @@ -200,6 +205,13 @@ def mock_connection_error(monkeypatch):
monkeypatch.setattr("requests.Session.send", MockConnectionError)


@pytest.fixture
def mock_retry_error(monkeypatch):
monkeypatch.setattr("requests.post", MockRetryError)
monkeypatch.setattr("requests.get", MockRetryError)
monkeypatch.setattr("requests.Session.send", MockRetryError)


@pytest.fixture
def mock_token(mock_credentials, mock_successful_post_token_response):
return WorldcatAccessToken(**mock_credentials)
Expand All @@ -211,6 +223,18 @@ def stub_session(mock_token):
yield session


@pytest.fixture
def stub_retry_session(mock_token):
with MetadataSession(
authorization=mock_token,
total_retries=3,
backoff_factor=0.5,
status_forcelist=[500, 502, 503, 504],
allowed_methods=["GET", "POST", "PUT"],
) as session:
yield session


@pytest.fixture
def mock_400_response(monkeypatch):
def mock_api_response(*args, **kwargs):
Expand Down
34 changes: 34 additions & 0 deletions tests/test_metadata_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1194,3 +1194,37 @@ def test_validate_bib(self, live_keys, stub_marc21):
== "https://metadata.api.oclc.org/worldcat/manage/bibs/validate/validateFull"
)
assert sorted(response.json().keys()) == sorted(["httpStatus", "status"])

def test_default_retries(self, live_keys, stub_marc21):
token = WorldcatAccessToken(
key=os.getenv("WCKey"),
secret=os.getenv("WCSecret"),
scopes=os.getenv("WCScopes"),
)

with MetadataSession(authorization=token) as session:
with pytest.raises(WorldcatRequestError) as exc:
session.validate_bib(stub_marc21, recordFormat="foo/bar")
assert "406 Client Error: Not Acceptable for url: " in (str(exc.value))
assert session.adapters["https://"].max_retries.total == 0

def test_custom_retries(self, live_keys, stub_marc21):
token = WorldcatAccessToken(
key=os.getenv("WCKey"),
secret=os.getenv("WCSecret"),
scopes=os.getenv("WCScopes"),
)

with MetadataSession(
authorization=token,
total_retries=3,
backoff_factor=0.5,
status_forcelist=[406],
allowed_methods=["GET", "POST"],
) as session:
with pytest.raises(WorldcatRequestError) as exc:
session.validate_bib(stub_marc21, recordFormat="foo/bar")
assert "Connection Error: <class 'requests.exceptions.RetryError'>" in (
str(exc.value)
)
assert session.adapters["https://"].max_retries.total == 3
37 changes: 33 additions & 4 deletions tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_query_live(live_keys):
)
with MetadataSession(authorization=token) as session:
header = {"Accept": "application/json"}
url = "https://americas.metadata.api.oclc.org/worldcat/search/v1/brief-bibs/41266045"
url = "https://metadata.api.oclc.org/worldcat/search/brief-bibs/41266045"
req = Request(
"GET",
url,
Expand Down Expand Up @@ -95,9 +95,7 @@ def test_query_http_207_response(stub_session, mock_session_response):
@pytest.mark.http_code(404)
def test_query_http_404_response(stub_session, mock_session_response):
header = {"Accept": "application/json"}
url = (
"https://americas.metadata.api.oclc.org/worldcat/search/v1/brief-bibs/41266045"
)
url = "https://metadata.api.oclc.org/worldcat/search/brief-bibs/41266045"
req = Request("GET", url, headers=header, hooks=None)
prepped = stub_session.prepare_request(req)

Expand Down Expand Up @@ -143,10 +141,41 @@ def test_query_connection_exception(stub_session, mock_connection_error):
)


def test_query_retry_exception(stub_session, mock_retry_error):
req = Request("GET", "https://foo.org")
prepped = stub_session.prepare_request(req)
with pytest.raises(WorldcatRequestError) as exc:
Query(stub_session, prepped)

assert "Connection Error: <class 'requests.exceptions.RetryError'>" in str(
exc.value
)


def test_query_unexpected_exception(stub_session, mock_unexpected_error):
req = Request("GET", "https://foo.org")
prepped = stub_session.prepare_request(req)
with pytest.raises(WorldcatRequestError) as exc:
Query(stub_session, prepped)

assert "Unexpected request error: <class 'Exception'>" in str(exc.value)


def test_query_timeout_retry(stub_retry_session, caplog):
req = Request("GET", "https://foo.org")
prepped = stub_retry_session.prepare_request(req)
with pytest.raises(WorldcatRequestError):
Query(stub_retry_session, prepped)

assert "Retry(total=0, " in caplog.records[2].message
assert "Retry(total=1, " in caplog.records[1].message
assert "Retry(total=2, " in caplog.records[0].message


def test_query_timeout_no_retry(stub_session, caplog):
req = Request("GET", "https://foo.org")
prepped = stub_session.prepare_request(req)
with pytest.raises(WorldcatRequestError):
Query(stub_session, prepped)

assert "Retry" not in caplog.records
Comment on lines +164 to +181
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 Awesome!

19 changes: 19 additions & 0 deletions tests/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,22 @@ def test_default_timeout(self, mock_token):
def test_custom_timeout(self, mock_token):
with WorldcatSession(mock_token, timeout=1) as session:
assert session.timeout == 1

def test_default_adapter(self, mock_token):
with WorldcatSession(mock_token) as session:
assert session.adapters["https://"].max_retries.total == 0

def test_adapter_retries(self, mock_token):
with WorldcatSession(
authorization=mock_token,
total_retries=3,
backoff_factor=0.5,
status_forcelist=[500, 502, 503, 504],
allowed_methods=["GET", "POST", "PUT"],
) as session:
assert session.adapters["https://"].max_retries.status_forcelist == [
500,
502,
503,
504,
]
Comment on lines +41 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!