fix: revert "feat: add timeout to AuthorizedSession.request() (#397)" (#401)

This reverts commit 381dd400911d29926ffbf04e0f2ba53ef7bb997e.

Author: busunkim96

packages/google-auth/google/auth/transport/requests.py

@@ -18,7 +18,6 @@
 
 import functools
 import logging
-import time
 
 try:
     import requests
@@ -65,33 +64,6 @@ def data(self):
         return self._response.content
 
 
-class TimeoutGuard(object):
-    """A context manager raising an error if the suite execution took too long.
-    """
-
-    def __init__(self, timeout, timeout_error_type=requests.exceptions.Timeout):
-        self._timeout = timeout
-        self.remaining_timeout = timeout
-        self._timeout_error_type = timeout_error_type
-
-    def __enter__(self):
-        self._start = time.time()
-        return self
-
-    def __exit__(self, exc_type, exc_value, traceback):
-        if exc_value:
-            return  # let the error bubble up automatically
-
-        if self._timeout is None:
-            return  # nothing to do, the timeout was not specified
-
-        elapsed = time.time() - self._start
-        self.remaining_timeout = self._timeout - elapsed
-
-        if self.remaining_timeout <= 0:
-            raise self._timeout_error_type()
-
-
 class Request(transport.Request):
     """Requests request adapter.
 
@@ -221,12 +193,8 @@ def __init__(
         # credentials.refresh).
         self._auth_request = auth_request
 
-    def request(self, method, url, data=None, headers=None, timeout=None, **kwargs):
-        """Implementation of Requests' request.
-
-        The ``timeout`` argument is interpreted as the approximate total time
-        of **all** requests that are made under the hood.
-        """
+    def request(self, method, url, data=None, headers=None, **kwargs):
+        """Implementation of Requests' request."""
         # pylint: disable=arguments-differ
         # Requests has a ton of arguments to request, but only two
         # (method, url) are required. We pass through all of the other
@@ -240,28 +208,13 @@ def request(self, method, url, data=None, headers=None, timeout=None, **kwargs):
         # and we want to pass the original headers if we recurse.
         request_headers = headers.copy() if headers is not None else {}
 
-        # Do not apply the timeout unconditionally in order to not override the
-        # _auth_request's default timeout.
-        auth_request = (
-            self._auth_request
-            if timeout is None
-            else functools.partial(self._auth_request, timeout=timeout)
+        self.credentials.before_request(
+            self._auth_request, method, url, request_headers
         )
 
-        with TimeoutGuard(timeout) as guard:
-            self.credentials.before_request(auth_request, method, url, request_headers)
-        timeout = guard.remaining_timeout
-
-        with TimeoutGuard(timeout) as guard:
-            response = super(AuthorizedSession, self).request(
-                method,
-                url,
-                data=data,
-                headers=request_headers,
-                timeout=timeout,
-                **kwargs
-            )
-        timeout = guard.remaining_timeout
+        response = super(AuthorizedSession, self).request(
+            method, url, data=data, headers=request_headers, **kwargs
+        )
 
         # If the response indicated that the credentials needed to be
         # refreshed, then refresh the credentials and re-attempt the
@@ -280,33 +233,17 @@ def request(self, method, url, data=None, headers=None, timeout=None, **kwargs):
                 self._max_refresh_attempts,
             )
 
-            if self._refresh_timeout is not None:
-                timeout = (
-                    self._refresh_timeout
-                    if timeout is None
-                    else min(timeout, self._refresh_timeout)
-                )
-
-            # Do not apply the timeout unconditionally in order to not override the
-            # _auth_request's default timeout.
-            auth_request = (
-                self._auth_request
-                if timeout is None
-                else functools.partial(self._auth_request, timeout=timeout)
+            auth_request_with_timeout = functools.partial(
+                self._auth_request, timeout=self._refresh_timeout
             )
+            self.credentials.refresh(auth_request_with_timeout)
 
-            with TimeoutGuard(timeout) as guard:
-                self.credentials.refresh(auth_request)
-            timeout = guard.remaining_timeout
-
-            # Recurse. Pass in the original headers, not our modified set, but
-            # do pass the adjusted timeout (i.e. the remaining time).
+            # Recurse. Pass in the original headers, not our modified set.
             return self.request(
                 method,
                 url,
                 data=data,
                 headers=headers,
-                timeout=timeout,
                 _credential_refresh_attempt=_credential_refresh_attempt + 1,
                 **kwargs
             )

packages/google-auth/noxfile.py

@@ -16,7 +16,6 @@
 
 TEST_DEPENDENCIES = [
     "flask",
-    "freezegun",
     "mock",
     "oauth2client",
     "pytest",

packages/google-auth/tests/transport/test_requests.py

@@ -12,12 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import datetime
-import functools
-
-import freezegun
 import mock
-import pytest
 import requests
 import requests.adapters
 from six.moves import http_client
@@ -27,12 +22,6 @@
 from tests.transport import compliance
 
 
-@pytest.fixture
-def frozen_time():
-    with freezegun.freeze_time("1970-01-01 00:00:00", tick=False) as frozen:
-        yield frozen
-
-
 class TestRequestResponse(compliance.RequestResponseTests):
     def make_request(self):
         return google.auth.transport.requests.Request()
@@ -45,41 +34,6 @@ def test_timeout(self):
         assert http.request.call_args[1]["timeout"] == 5
 
 
-class TestTimeoutGuard(object):
-    def make_guard(self, *args, **kwargs):
-        return google.auth.transport.requests.TimeoutGuard(*args, **kwargs)
-
-    def test_tracks_elapsed_time(self, frozen_time):
-        with self.make_guard(timeout=10) as guard:
-            frozen_time.tick(delta=3.8)
-        assert guard.remaining_timeout == 6.2
-
-    def test_noop_if_no_timeout(self, frozen_time):
-        with self.make_guard(timeout=None) as guard:
-            frozen_time.tick(delta=datetime.timedelta(days=3650))
-        # NOTE: no timeout error raised, despite years have passed
-        assert guard.remaining_timeout is None
-
-    def test_error_on_timeout(self, frozen_time):
-        with pytest.raises(requests.exceptions.Timeout):
-            with self.make_guard(timeout=10) as guard:
-                frozen_time.tick(delta=10.001)
-        assert guard.remaining_timeout == pytest.approx(-0.001)
-
-    def test_custom_timeout_error_type(self, frozen_time):
-        class FooError(Exception):
-            pass
-
-        with pytest.raises(FooError):
-            with self.make_guard(timeout=1, timeout_error_type=FooError):
-                frozen_time.tick(2)
-
-    def test_lets_errors_bubble_up(self, frozen_time):
-        with pytest.raises(IndexError):
-            with self.make_guard(timeout=1):
-                [1, 2, 3][3]
-
-
 class CredentialsStub(google.auth.credentials.Credentials):
     def __init__(self, token="token"):
         super(CredentialsStub, self).__init__()
@@ -95,18 +49,6 @@ def refresh(self, request):
         self.token += "1"
 
 
-class TimeTickCredentialsStub(CredentialsStub):
-    """Credentials that spend some (mocked) time when refreshing a token."""
-
-    def __init__(self, time_tick, token="token"):
-        self._time_tick = time_tick
-        super(TimeTickCredentialsStub, self).__init__(token=token)
-
-    def refresh(self, request):
-        self._time_tick()
-        super(TimeTickCredentialsStub, self).refresh(requests)
-
-
 class AdapterStub(requests.adapters.BaseAdapter):
     def __init__(self, responses, headers=None):
         super(AdapterStub, self).__init__()
@@ -127,18 +69,6 @@ def close(self):  # pragma: NO COVER
         return
 
 
-class TimeTickAdapterStub(AdapterStub):
-    """Adapter that spends some (mocked) time when making a request."""
-
-    def __init__(self, time_tick, responses, headers=None):
-        self._time_tick = time_tick
-        super(TimeTickAdapterStub, self).__init__(responses, headers=headers)
-
-    def send(self, request, **kwargs):
-        self._time_tick()
-        return super(TimeTickAdapterStub, self).send(request, **kwargs)
-
-
 def make_response(status=http_client.OK, data=None):
     response = requests.Response()
     response.status_code = status
@@ -191,9 +121,7 @@ def test_request_refresh(self):
             [make_response(status=http_client.UNAUTHORIZED), final_response]
         )
 
-        authed_session = google.auth.transport.requests.AuthorizedSession(
-            credentials, refresh_timeout=60
-        )
+        authed_session = google.auth.transport.requests.AuthorizedSession(credentials)
         authed_session.mount(self.TEST_URL, adapter)
 
         result = authed_session.request("GET", self.TEST_URL)
@@ -208,44 +136,3 @@ def test_request_refresh(self):
 
         assert adapter.requests[1].url == self.TEST_URL
         assert adapter.requests[1].headers["authorization"] == "token1"
-
-    def test_request_timout(self, frozen_time):
-        tick_one_second = functools.partial(frozen_time.tick, delta=1.0)
-
-        credentials = mock.Mock(
-            wraps=TimeTickCredentialsStub(time_tick=tick_one_second)
-        )
-        adapter = TimeTickAdapterStub(
-            time_tick=tick_one_second,
-            responses=[
-                make_response(status=http_client.UNAUTHORIZED),
-                make_response(status=http_client.OK),
-            ],
-        )
-
-        authed_session = google.auth.transport.requests.AuthorizedSession(credentials)
-        authed_session.mount(self.TEST_URL, adapter)
-
-        # Because at least two requests have to be made, and each takes one
-        # second, the total timeout specified will be exceeded.
-        with pytest.raises(requests.exceptions.Timeout):
-            authed_session.request("GET", self.TEST_URL, timeout=1.9)
-
-    def test_request_timeout_w_refresh_timeout(self, frozen_time):
-        credentials = mock.Mock(wraps=CredentialsStub())
-        adapter = TimeTickAdapterStub(
-            time_tick=functools.partial(frozen_time.tick, delta=1.0),  # one second
-            responses=[
-                make_response(status=http_client.UNAUTHORIZED),
-                make_response(status=http_client.OK),
-            ],
-        )
-
-        authed_session = google.auth.transport.requests.AuthorizedSession(
-            credentials, refresh_timeout=0.9
-        )
-        authed_session.mount(self.TEST_URL, adapter)
-
-        # The timeout is long, but the short refresh timeout will prevail.
-        with pytest.raises(requests.exceptions.Timeout):
-            authed_session.request("GET", self.TEST_URL, timeout=60)