Merge pull request #97 from praw-dev/last_request

Replace uses of time.time with time.monotonic(_ns)

Author: LilSpazJoekp

CHANGES.rst

@@ -9,6 +9,18 @@ Unreleased
 **Changed**
 
 - Drop support for Python 3.8, which was end-of-life on 2024-10-07.
+- :class:`RateLimiter` attribute ``next_request_timestamp`` has been removed and
+  replaced with ``next_request_timestamp_ns``.
+
+**Fixed**
+
+- Add a half-second delay when there are no more requests in the rate limit window and
+  the window has zero seconds remaining to avoid a semi-rare case where Reddit will
+  return a 429 response resulting in a :class:`TooManyRequests` exception.
+
+**Removed**
+
+- Remove :class:`RateLimiter` attribute ``reset_timestamp``.
 
 2.4.0 (2023/11/27)
 ------------------

asyncprawcore/auth.py

@@ -150,21 +150,23 @@ def __init__(self, authenticator: BaseAuthenticator):
         self._validate_authenticator()
 
     def _clear_access_token(self):
-        self._expiration_timestamp: float
+        self._expiration_timestamp_ns: int
         self.access_token: str | None = None
         self.scopes: set[str] | None = None
 
     async def _request_token(self, **data: Any):
         url = self._authenticator._requestor.reddit_url + const.ACCESS_TOKEN_PATH
-        pre_request_time = time.time()
+        pre_request_timestamp_ns = time.monotonic_ns()
         async with self._authenticator._post(url=url, **data) as response:
             payload = await response.json()
         if "error" in payload:  # Why are these OKAY responses?
             raise OAuthException(
                 response, payload["error"], payload.get("error_description")
             )
 
-        self._expiration_timestamp = pre_request_time - 10 + payload["expires_in"]
+        self._expiration_timestamp_ns = (
+            pre_request_timestamp_ns + (payload["expires_in"] + 10) * const.NANOSECONDS
+        )
         self.access_token = payload["access_token"]
         if "refresh_token" in payload:
             self.refresh_token = payload["refresh_token"]
@@ -189,7 +191,8 @@ def is_valid(self) -> bool:
 
         """
         return (
-            self.access_token is not None and time.time() < self._expiration_timestamp
+            self.access_token is not None
+            and time.monotonic_ns() < self._expiration_timestamp_ns
         )
 
     async def revoke(self):
@@ -360,7 +363,9 @@ def __init__(
 
         """
         super().__init__(authenticator)
-        self._expiration_timestamp = time.time() + expires_in
+        self._expiration_timestamp_ns = (
+            time.monotonic_ns() + expires_in * const.NANOSECONDS
+        )
         self.access_token = access_token
         self.scopes = set(scope.split(" "))
 

asyncprawcore/const.py

@@ -4,6 +4,7 @@
 
 ACCESS_TOKEN_PATH = "/api/v1/access_token"  # noqa: S105
 AUTHORIZATION_PATH = "/api/v1/authorize"  # noqa: S105
+NANOSECONDS = 1_000_000_000  # noqa: S105
 REVOKE_TOKEN_PATH = "/api/v1/revoke_token"  # noqa: S105
 TIMEOUT = float(
     os.environ.get(

asyncprawcore/rate_limit.py

@@ -13,6 +13,8 @@
 
     from aiohttp import ClientResponse
 
+from asyncprawcore.const import NANOSECONDS
+
 log = logging.getLogger(__package__)
 
 
@@ -23,8 +25,6 @@ class RateLimiter:
 
     """
 
-    NANOSECONDS = 1_000_000_000
-
     def __init__(self, *, window_size: int):
         """Create an instance of the RateLimit class."""
         self.remaining: int | None = None
@@ -61,8 +61,7 @@ async def delay(self):
         if self.next_request_timestamp_ns is None:
             return
         sleep_seconds = (
-            float(self.next_request_timestamp_ns - time.monotonic_ns())
-            / self.NANOSECONDS
+            float(self.next_request_timestamp_ns - time.monotonic_ns()) / NANOSECONDS
         )
         if sleep_seconds <= 0:
             return
@@ -94,7 +93,7 @@ def update(self, response_headers: Mapping[str, str]):
 
         if self.remaining <= 0:
             self.next_request_timestamp_ns = now_ns + max(
-                self.NANOSECONDS / 2, seconds_to_reset * self.NANOSECONDS
+                NANOSECONDS / 2, seconds_to_reset * NANOSECONDS
             )
             return
 
@@ -114,5 +113,5 @@ def update(self, response_headers: Mapping[str, str]):
                 ),
                 10,
             )
-            * self.NANOSECONDS
+            * NANOSECONDS
         )

asyncprawcore/sessions.py

@@ -111,7 +111,7 @@ def _log_request(
         params: dict[str, int],
         url: str,
     ):
-        log.debug("Fetching: %s %s at %s", method, url, time.time())
+        log.debug("Fetching: %s %s at %s", method, url, time.monotonic())
         log.debug("Data: %s", pformat(data))
         log.debug("Params: %s", pformat(params))
 
@@ -212,7 +212,7 @@ async def _make_request(
                     response.headers.get("x-ratelimit-reset"),
                     response.headers.get("x-ratelimit-remaining"),
                     response.headers.get("x-ratelimit-used"),
-                    time.time(),
+                    time.monotonic(),
                 )
                 yield response, None
         except RequestException as exception:

tests/unit/test_rate_limit.py

@@ -5,6 +5,7 @@
 
 import pytest
 
+from asyncprawcore.const import NANOSECONDS
 from asyncprawcore.rate_limit import RateLimiter
 
 from . import UnitTest
@@ -14,7 +15,7 @@ class TestRateLimiter(UnitTest):
     @pytest.fixture
     def rate_limiter(self):
         rate_limiter = RateLimiter(window_size=600)
-        rate_limiter.next_request_timestamp_ns = 100 * RateLimiter.NANOSECONDS
+        rate_limiter.next_request_timestamp_ns = 100 * NANOSECONDS
         return rate_limiter
 
     @staticmethod
@@ -28,7 +29,7 @@ def _headers(remaining, used, reset):
     @patch("time.monotonic_ns")
     @patch("asyncio.sleep")
     async def test_delay(self, mock_sleep, mock_monotonic_ns, rate_limiter):
-        mock_monotonic_ns.return_value = 1 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 1 * NANOSECONDS
         await rate_limiter.delay()
         assert mock_monotonic_ns.called
         mock_sleep.assert_called_with(99)
@@ -38,7 +39,7 @@ async def test_delay(self, mock_sleep, mock_monotonic_ns, rate_limiter):
     async def test_delay__no_sleep_when_time_in_past(
         self, mock_sleep, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 101 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 101 * NANOSECONDS
         await rate_limiter.delay()
         assert mock_monotonic_ns.called
         assert not mock_sleep.called
@@ -54,7 +55,7 @@ async def test_delay__no_sleep_when_time_is_not_set(self, mock_sleep, rate_limit
     async def test_delay__no_sleep_when_times_match(
         self, mock_sleep, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 100 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 100 * NANOSECONDS
         await rate_limiter.delay()
         assert mock_monotonic_ns.called
         assert not mock_sleep.called
@@ -63,64 +64,64 @@ async def test_delay__no_sleep_when_times_match(
     def test_update__compute_delay_with_no_previous_info(
         self, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 100 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 100 * NANOSECONDS
         rate_limiter.update(self._headers(60, 100, 60))
         assert rate_limiter.remaining == 60
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 100 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 100 * NANOSECONDS
 
     @patch("time.monotonic_ns")
     def test_update__compute_delay_with_single_client(
         self, mock_monotonic_ns, rate_limiter
     ):
         rate_limiter.window_size = 150
-        mock_monotonic_ns.return_value = 100 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 100 * NANOSECONDS
         rate_limiter.update(self._headers(50, 100, 60))
         assert rate_limiter.remaining == 50
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 110 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 110 * NANOSECONDS
 
     @patch("time.monotonic_ns")
     def test_update__compute_delay_with_six_clients(
         self, mock_monotonic_ns, rate_limiter
     ):
         rate_limiter.remaining = 66
         rate_limiter.window_size = 180
-        mock_monotonic_ns.return_value = 100 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 100 * NANOSECONDS
         rate_limiter.update(self._headers(60, 100, 72))
         assert rate_limiter.remaining == 60
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 104.5 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 104.5 * NANOSECONDS
 
     @patch("time.monotonic_ns")
     def test_update__delay_full_time_with_negative_remaining(
         self, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 37 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 37 * NANOSECONDS
         rate_limiter.update(self._headers(0, 100, 13))
         assert rate_limiter.remaining == 0
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 50 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 50 * NANOSECONDS
 
     @patch("time.monotonic_ns")
     def test_update__delay_full_time_with_zero_remaining(
         self, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 37 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 37 * NANOSECONDS
         rate_limiter.update(self._headers(0, 100, 13))
         assert rate_limiter.remaining == 0
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 50 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 50 * NANOSECONDS
 
     @patch("time.monotonic_ns")
     def test_update__delay_full_time_with_zero_remaining_and_no_sleep_time(
         self, mock_monotonic_ns, rate_limiter
     ):
-        mock_monotonic_ns.return_value = 37 * RateLimiter.NANOSECONDS
+        mock_monotonic_ns.return_value = 37 * NANOSECONDS
         rate_limiter.update(self._headers(0, 100, 0))
         assert rate_limiter.remaining == 0
         assert rate_limiter.used == 100
-        assert rate_limiter.next_request_timestamp_ns == 37.5 * RateLimiter.NANOSECONDS
+        assert rate_limiter.next_request_timestamp_ns == 37.5 * NANOSECONDS
 
     def test_update__no_change_without_headers(self, rate_limiter):
         prev = copy(rate_limiter)