Fixed inconsistency in timeout types/default values (#98)

* fixed inconsistency in timeout default values

* type fixes in WorldcatAccessToken

* test refactor, fixed typos and timeout assignment

* fixed timeout in Query, added file for live tests

bookops_worldcat/_session.py

@@ -35,8 +35,10 @@ def __init__(
             authorization:          WorldcatAccessToken instance
             agent:                  "User-agent" parameter to attached to each
                                     request in the session
-            timeout:                how long to wait for server to send data
-                                    before giving up
+            timeout:                how long to wait for server to send data before
+                                    giving up; can accept different values for connect
+                                    and read timeouts. default value is 5 seconds for
+                                    read and 5 seconds for connect timeouts
             totalRetries:           optional number of times to retry a request that
                                     failed or timed out. if totalRetries argument is
                                     not passed, any arguments passed to

bookops_worldcat/authorize.py

@@ -40,7 +40,9 @@ class WorldcatAccessToken:
         agent:                  "User-agent" parameter to be passed in the request
                                 header; usage strongly encouraged
         timeout:                how long to wait for server to send data before
-                                giving up; default value is 3 seconds
+                                giving up; can accept different values for connect
+                                and read timeouts. default value is 5 seconds for
+                                read and 5 seconds for connect timeouts
 
 
     Examples:
@@ -80,9 +82,10 @@ def __init__(
         secret: str,
         scopes: str,
         agent: str = "",
-        timeout: Optional[
-            Union[int, float, Tuple[int, int], Tuple[float, float]]
-        ] = None,
+        timeout: Union[int, float, Tuple[int, int], Tuple[float, float], None] = (
+            5,
+            5,
+        ),
     ) -> None:
         """Constructor"""
 
@@ -126,10 +129,6 @@ def __init__(
             raise TypeError("Argument 'scopes' must a string.")
         self.scopes = self.scopes.strip()
 
-        # assign default value for timout
-        if not self.timeout:
-            self.timeout = (3, 3)
-
         # initiate request
         self._request_token()
 

bookops_worldcat/metadata_api.py

@@ -23,7 +23,10 @@ def __init__(
         self,
         authorization: WorldcatAccessToken,
         agent: Optional[str] = None,
-        timeout: Union[int, float, Tuple[int, int], Tuple[float, float], None] = None,
+        timeout: Union[int, float, Tuple[int, int], Tuple[float, float], None] = (
+            5,
+            5,
+        ),
         totalRetries: int = 0,
         backoffFactor: float = 0,
         statusForcelist: Optional[List[int]] = None,
@@ -36,7 +39,9 @@ def __init__(
             agent:                  "User-agent" parameter to be passed in the request
                                     header; usage strongly encouraged
             timeout:                how long to wait for server to send data before
-                                    giving up; default value is 5 seconds
+                                    giving up; can accept different values for connect
+                                    and read timeouts. default value is 5 seconds for
+                                    read and 5 seconds for connect timeouts
             totalRetries:           optional number of times to retry a request that
                                     failed or timed out. if totalRetries argument is
                                     not passed, any arguments passed to

bookops_worldcat/query.py

@@ -4,7 +4,7 @@
 Handles actual requests to OCLC services
 """
 from __future__ import annotations
-from typing import Optional, Union, Tuple, TYPE_CHECKING
+from typing import Union, Tuple, TYPE_CHECKING
 import sys
 
 from requests.models import PreparedRequest
@@ -31,17 +31,20 @@ def __init__(
         self,
         session: MetadataSession,
         prepared_request: PreparedRequest,
-        timeout: Optional[
-            Union[int, float, Tuple[int, int], Tuple[float, float]]
-        ] = None,
+        timeout: Union[int, float, Tuple[int, int], Tuple[float, float], None] = (
+            5,
+            5,
+        ),
     ) -> None:
         """Initializes Query object.
 
         Args:
-            session:                        `metadata_api.MetadataSession` instance
-            prepared_request:               `requests.models.PreparedRequest` instance
-            timeout:                        how long to wait for server to send data
-                                            before giving up
+            session:                `metadata_api.MetadataSession` instance
+            prepared_request:       `requests.models.PreparedRequest` instance
+            timeout:                how long to wait for server to send data before
+                                    giving up; can accept different values for connect
+                                    and read timeouts. default value is 5 seconds for
+                                    read and 5 seconds for connect timeouts
 
 
         Raises:

tests/conftest.py

@@ -38,7 +38,7 @@ def stub_holding_xml():
 @pytest.fixture
 def stub_marc21():
     fh = os.path.join(
-        os.environ["USERPROFILE"], "github/bookops-worldcat/temp/test.mrc"
+        os.environ["USERPROFILE"], "github/bookops-worldcat/tests/test.mrc"
     )
     with open(fh, "rb") as stub:
         stub_marc21 = stub.read()

tests/test.mrc

@@ -0,0 +1 @@
+00266nam a2200097 a 4500008004100000010001700041040002200058100002700080245001600107500004500123120827s2012    nyua          000 0 eng d  a   63011276   aOCWMSbengcOCWMS0 aOCLC Developer Network10aTest Record  aFOR OCLC DEVELOPER NETWORK DOCUMENTATION

tests/test_authorize.py

@@ -7,7 +7,6 @@
 
 
 from bookops_worldcat.authorize import WorldcatAccessToken
-from bookops_worldcat import __version__, __title__
 from bookops_worldcat.errors import WorldcatAuthorizationError
 
 
@@ -119,7 +118,7 @@ def test_scope_exceptions(self, argm, expectation, msg):
     @pytest.mark.parametrize(
         "argm,expectation",
         [
-            (None, (3, 3)),
+            (None, None),
             (1, 1),
             (0.5, 0.5),
             ((5, 5), (5, 5)),
@@ -201,7 +200,7 @@ def test_payload(self, mock_successful_post_token_response):
             "scope": "scope1",
         }
 
-    def test_post_token_request_timout(self, mock_credentials, mock_timeout):
+    def test_post_token_request_timeout(self, mock_credentials, mock_timeout):
         creds = mock_credentials
         with pytest.raises(WorldcatAuthorizationError):
             WorldcatAccessToken(
@@ -291,7 +290,7 @@ def test_post_token_request(
         assert token.oauth_server == "https://oauth.oclc.org"
         assert token.scopes == "scope1 scope2"
         assert token.server_response.json() == mock_oauth_server_response.json()
-        assert token.timeout == (3, 3)
+        assert token.timeout == (5, 5)
 
     def test_token_repr(
         self,
@@ -342,3 +341,38 @@ def test_post_token_request_with_live_service(self, live_keys):
         assert token.token_str.startswith("tk_")
         assert token.is_expired() is False
         assert isinstance(token.token_expires_at, datetime.datetime)
+
+    @pytest.mark.webtest
+    def test_post_token_request_with_live_service_no_timeout(self, live_keys):
+        token = WorldcatAccessToken(
+            key=os.getenv("WCKey"),
+            secret=os.getenv("WCSecret"),
+            scopes=os.getenv("WCScopes"),
+            timeout=None,
+        )
+
+        assert token.server_response.status_code == 200
+
+        # test presence of all returned parameters
+        response = token.server_response.json()
+        params = [
+            "access_token",
+            "expires_at",
+            "authenticating_institution_id",
+            "principalID",
+            "context_institution_id",
+            "scopes",
+            "token_type",
+            "expires_in",
+            "principalIDNS",
+        ]
+        for p in params:
+            assert p in response
+
+        # test if any new additions are present
+        assert sorted(params) == sorted(response.keys())
+
+        # test if token looks right
+        assert token.token_str.startswith("tk_")
+        assert token.is_expired() is False
+        assert isinstance(token.token_expires_at, datetime.datetime)