From 2255b56bcbb50a01ee360b95be90f02140c15421 Mon Sep 17 00:00:00 2001 From: Allan Beaufour Date: Sun, 25 Jan 2026 12:03:03 -0500 Subject: [PATCH 1/4] Add design doc for proactive rate limiting Documents the approach for opt-in rate limiting to prevent exceeding Flickr's 3600 queries/hour limit. Complements existing 429 retry handling. Co-Authored-By: Claude Opus 4.5 --- ...26-01-25-proactive-rate-limiting-design.md | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 docs/plans/2026-01-25-proactive-rate-limiting-design.md diff --git a/docs/plans/2026-01-25-proactive-rate-limiting-design.md b/docs/plans/2026-01-25-proactive-rate-limiting-design.md new file mode 100644 index 0000000..e0c3068 --- /dev/null +++ b/docs/plans/2026-01-25-proactive-rate-limiting-design.md @@ -0,0 +1,92 @@ +# Proactive Rate Limiting Design + +## Overview + +Add opt-in rate limiting to prevent exceeding Flickr's 3600 queries/hour limit. This complements the existing reactive rate limit handling (HTTP 429 retry with backoff). + +## Public API + +```python +# Enable rate limiting (3600 requests/hour = ~1 request/second) +flickr_api.set_rate_limit(requests_per_hour=3600) + +# Disable rate limiting +flickr_api.set_rate_limit(requests_per_hour=None) + +# Get current setting +flickr_api.get_rate_limit() +# Returns: {"requests_per_hour": 3600} or {"requests_per_hour": None} + +# Get status including last request time +flickr_api.get_rate_limit_status() +# Returns: { +# "enabled": True, +# "requests_per_hour": 3600, +# "interval_seconds": 1.0, +# "last_request_time": 1706198400.123 # or None if no requests yet +# } +``` + +## Design Decisions + +1. **Simple sleep-based throttling** - Sleep a calculated interval between requests. Simple and predictable. + +2. **Opt-in** - Disabled by default to preserve backward compatibility. + +3. **Requests-per-hour configuration** - Maps directly to Flickr's documented limit (3600/hour). Internally converted to interval: `interval = 3600 / requests_per_hour`. + +4. **Basic status API** - Provides enough info for debugging without over-engineering. + +## Implementation + +### Module Globals (method_call.py) + +```python +_RATE_LIMIT_REQUESTS_PER_HOUR: float | None = None # None = disabled +_RATE_LIMIT_LAST_REQUEST: float | None = None # timestamp of last request +``` + +### Throttling Function + +```python +def _maybe_wait_for_rate_limit() -> None: + """Sleep if necessary to respect rate limit.""" + global _RATE_LIMIT_LAST_REQUEST + + if _RATE_LIMIT_REQUESTS_PER_HOUR is None: + return # Rate limiting disabled + + interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR + + if _RATE_LIMIT_LAST_REQUEST is not None: + elapsed = time.time() - _RATE_LIMIT_LAST_REQUEST + if elapsed < interval: + time.sleep(interval - elapsed) + + _RATE_LIMIT_LAST_REQUEST = time.time() +``` + +### Integration Point + +Called at the start of `_make_request_with_retry()`, before making each HTTP request. + +## Files to Modify + +- `flickr_api/method_call.py` - Add rate limit globals and functions, integrate into request flow +- `flickr_api/__init__.py` - Export new functions +- `test/test_rate_limit_throttle.py` - New test file + +## Test Plan + +1. `test_rate_limit_disabled_by_default` - Verify disabled initially +2. `test_set_and_get_rate_limit` - Set/get round-trip +3. `test_disable_rate_limit` - Set to None disables +4. `test_interval_calculation` - 3600 req/hour = 1.0s, 1800 = 2.0s +5. `test_get_rate_limit_status` - Status dict structure +6. `test_no_sleep_when_disabled` - No sleep when off +7. `test_sleeps_when_interval_not_elapsed` - Correct sleep duration +8. `test_no_sleep_when_interval_elapsed` - No sleep if enough time passed +9. `test_first_request_no_sleep` - First request doesn't wait +10. `test_updates_last_request_time` - Timestamp updates correctly + +Tests will mock `time.sleep` and `time.time` for deterministic behavior. From da22d4d8190117a028d2d28dac1e1070122d7e07 Mon Sep 17 00:00:00 2001 From: Allan Beaufour Date: Sun, 25 Jan 2026 12:06:25 -0500 Subject: [PATCH 2/4] Add proactive rate limiting to method_call.py Implement opt-in rate limiting that throttles requests to respect Flickr's documented 3600 requests/hour limit. This is proactive throttling (sleeping before requests) as opposed to the existing reactive retry handling (retrying after 429 responses). New public functions: - set_rate_limit(requests_per_hour) - Enable/disable rate limiting - get_rate_limit() - Get current configuration - get_rate_limit_status() - Get detailed status including interval Internal function _maybe_wait_for_rate_limit() is called at the start of _make_request_with_retry() to sleep if necessary before making each request. Co-Authored-By: Claude Opus 4.5 --- flickr_api/method_call.py | 85 ++++++++++ test/test_rate_limit_throttle.py | 261 +++++++++++++++++++++++++++++++ 2 files changed, 346 insertions(+) create mode 100644 test/test_rate_limit_throttle.py diff --git a/flickr_api/method_call.py b/flickr_api/method_call.py index 5f91254..ee224cb 100644 --- a/flickr_api/method_call.py +++ b/flickr_api/method_call.py @@ -36,6 +36,10 @@ RETRY_BASE_DELAY: float = 1.0 # Base delay in seconds for exponential backoff RETRY_MAX_DELAY: float = 60.0 # Maximum delay between retries +# Proactive rate limiting configuration +_RATE_LIMIT_REQUESTS_PER_HOUR: float | None = None +_RATE_LIMIT_LAST_REQUEST: float | None = None + def enable_cache(cache_object: Any | None = None) -> None: """enable caching @@ -109,6 +113,85 @@ def get_retry_config() -> dict[str, Any]: } +def set_rate_limit(requests_per_hour: float | None) -> None: + """Enable or disable proactive rate limiting. + + Parameters: + ----------- + requests_per_hour: float | None + Maximum requests per hour. Set to None to disable rate limiting. + Flickr's documented limit is 3600 requests per hour. + + Raises: + ------- + ValueError: If requests_per_hour is not positive (zero or negative). + """ + if requests_per_hour is not None and requests_per_hour <= 0: + raise ValueError("requests_per_hour must be positive") + global _RATE_LIMIT_REQUESTS_PER_HOUR + _RATE_LIMIT_REQUESTS_PER_HOUR = requests_per_hour + + +def get_rate_limit() -> dict[str, float | None]: + """Get current rate limit configuration. + + Returns: + -------- + dict with key: requests_per_hour (float | None) + """ + return {"requests_per_hour": _RATE_LIMIT_REQUESTS_PER_HOUR} + + +def get_rate_limit_status() -> dict[str, Any]: + """Get detailed rate limit status. + + Returns: + -------- + dict with keys: + - enabled: bool - Whether rate limiting is active + - requests_per_hour: float | None - Configured limit + - interval_seconds: float - Minimum time between requests (0.0 if disabled) + - last_request_time: float | None - Timestamp of last request + """ + enabled = _RATE_LIMIT_REQUESTS_PER_HOUR is not None + interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR if enabled else 0.0 + return { + "enabled": enabled, + "requests_per_hour": _RATE_LIMIT_REQUESTS_PER_HOUR, + "interval_seconds": interval, + "last_request_time": _RATE_LIMIT_LAST_REQUEST, + } + + +def _maybe_wait_for_rate_limit() -> None: + """Wait if necessary to respect rate limit. + + This function should be called before making a request. It will: + 1. Do nothing if rate limiting is disabled + 2. Do nothing if this is the first request + 3. Sleep for the remaining interval time if needed + 4. Update the last request timestamp + """ + global _RATE_LIMIT_LAST_REQUEST + + if _RATE_LIMIT_REQUESTS_PER_HOUR is None: + return + + current_time = time.time() + + if _RATE_LIMIT_LAST_REQUEST is not None: + interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR + elapsed = current_time - _RATE_LIMIT_LAST_REQUEST + remaining = interval - elapsed + + if remaining > 0: + logger.debug("Rate limiting: sleeping for %.2f seconds", remaining) + time.sleep(remaining) + current_time = time.time() + + _RATE_LIMIT_LAST_REQUEST = current_time + + def _calculate_retry_delay(attempt: int, retry_after: float | None) -> float: """Calculate delay before next retry. @@ -181,6 +264,8 @@ def _make_request_with_retry( ------- FlickrRateLimitError: If rate limit exceeded and max retries exhausted """ + _maybe_wait_for_rate_limit() + last_error: FlickrRateLimitError | None = None for attempt in range(MAX_RETRIES + 1): diff --git a/test/test_rate_limit_throttle.py b/test/test_rate_limit_throttle.py new file mode 100644 index 0000000..06f8b7d --- /dev/null +++ b/test/test_rate_limit_throttle.py @@ -0,0 +1,261 @@ +"""Tests for proactive rate limiting (throttling) in method_call module.""" + +import unittest +from unittest.mock import patch + +from flickr_api import method_call + + +class TestRateLimitDisabledByDefault(unittest.TestCase): + """Test that rate limiting is disabled by default.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_rate_limit_disabled_by_default(self): + """Rate limiting should be disabled by default.""" + result = method_call.get_rate_limit() + self.assertIsNone(result["requests_per_hour"]) + + +class TestSetAndGetRateLimit(unittest.TestCase): + """Test setting and getting rate limit configuration.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_set_and_get_rate_limit(self): + """Can set and get rate limit value.""" + method_call.set_rate_limit(3600.0) + result = method_call.get_rate_limit() + self.assertEqual(3600.0, result["requests_per_hour"]) + + def test_disable_rate_limit(self): + """Can disable rate limiting by setting to None.""" + method_call.set_rate_limit(3600.0) + method_call.set_rate_limit(None) + result = method_call.get_rate_limit() + self.assertIsNone(result["requests_per_hour"]) + + def test_reject_zero_rate_limit(self): + """Zero rate limit should raise ValueError.""" + with self.assertRaises(ValueError) as ctx: + method_call.set_rate_limit(0.0) + self.assertEqual(str(ctx.exception), "requests_per_hour must be positive") + + def test_reject_negative_rate_limit(self): + """Negative rate limit should raise ValueError.""" + with self.assertRaises(ValueError) as ctx: + method_call.set_rate_limit(-100.0) + self.assertEqual(str(ctx.exception), "requests_per_hour must be positive") + + +class TestIntervalCalculation(unittest.TestCase): + """Test interval calculation for rate limiting.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_interval_calculation_3600_per_hour(self): + """3600 requests/hour = 1.0 second interval.""" + method_call.set_rate_limit(3600.0) + status = method_call.get_rate_limit_status() + self.assertEqual(1.0, status["interval_seconds"]) + + def test_interval_calculation_1800_per_hour(self): + """1800 requests/hour = 2.0 second interval.""" + method_call.set_rate_limit(1800.0) + status = method_call.get_rate_limit_status() + self.assertEqual(2.0, status["interval_seconds"]) + + def test_interval_calculation_7200_per_hour(self): + """7200 requests/hour = 0.5 second interval.""" + method_call.set_rate_limit(7200.0) + status = method_call.get_rate_limit_status() + self.assertEqual(0.5, status["interval_seconds"]) + + +class TestGetRateLimitStatus(unittest.TestCase): + """Test get_rate_limit_status function.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def test_get_rate_limit_status_disabled(self): + """Status shows disabled state correctly.""" + status = method_call.get_rate_limit_status() + self.assertFalse(status["enabled"]) + self.assertIsNone(status["requests_per_hour"]) + self.assertEqual(0.0, status["interval_seconds"]) + self.assertIsNone(status["last_request_time"]) + + def test_get_rate_limit_status_enabled(self): + """Status shows enabled state correctly.""" + method_call.set_rate_limit(3600.0) + status = method_call.get_rate_limit_status() + self.assertTrue(status["enabled"]) + self.assertEqual(3600.0, status["requests_per_hour"]) + self.assertEqual(1.0, status["interval_seconds"]) + self.assertIsNone(status["last_request_time"]) + + def test_get_rate_limit_status_with_last_request(self): + """Status includes last request time when set.""" + method_call.set_rate_limit(3600.0) + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 + status = method_call.get_rate_limit_status() + self.assertEqual(1000.0, status["last_request_time"]) + + +class TestNoSleepWhenDisabled(unittest.TestCase): + """Test that no sleep occurs when rate limiting is disabled.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_no_sleep_when_disabled(self, mock_time, mock_sleep): + """No sleep when rate limiting is disabled.""" + method_call._maybe_wait_for_rate_limit() + mock_sleep.assert_not_called() + + +class TestSleepsWhenIntervalNotElapsed(unittest.TestCase): + """Test that sleep occurs when interval hasn't elapsed.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.5) + def test_sleeps_when_interval_not_elapsed(self, mock_time, mock_sleep): + """Sleep for remaining time when interval hasn't elapsed.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 # 0.5 seconds ago + + method_call._maybe_wait_for_rate_limit() + + # Should sleep for 0.5 seconds (1.0 - 0.5) + mock_sleep.assert_called_once_with(0.5) + + +class TestNoSleepWhenIntervalElapsed(unittest.TestCase): + """Test that no sleep occurs when interval has elapsed.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1002.0) + def test_no_sleep_when_interval_elapsed(self, mock_time, mock_sleep): + """No sleep when interval has already elapsed.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 # 2.0 seconds ago + + method_call._maybe_wait_for_rate_limit() + + mock_sleep.assert_not_called() + + +class TestFirstRequestNoSleep(unittest.TestCase): + """Test that first request doesn't sleep.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_first_request_no_sleep(self, mock_time, mock_sleep): + """First request (no last_request_time) doesn't sleep.""" + method_call.set_rate_limit(3600.0) # Rate limiting enabled + # _RATE_LIMIT_LAST_REQUEST is None (first request) + + method_call._maybe_wait_for_rate_limit() + + mock_sleep.assert_not_called() + + +class TestUpdatesLastRequestTime(unittest.TestCase): + """Test that last request time is updated after waiting.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_updates_last_request_time(self, mock_time, mock_sleep): + """Last request time is updated after _maybe_wait_for_rate_limit.""" + method_call.set_rate_limit(3600.0) + + method_call._maybe_wait_for_rate_limit() + + self.assertEqual(1000.0, method_call._RATE_LIMIT_LAST_REQUEST) + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1001.0) + def test_updates_last_request_time_after_sleep(self, mock_time, mock_sleep): + """Last request time is updated to current time after sleeping.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.5 # Would need to wait + + method_call._maybe_wait_for_rate_limit() + + # Should update to the current time after potential sleep + self.assertEqual(1001.0, method_call._RATE_LIMIT_LAST_REQUEST) From e4c7ee35f43b42b93b7c408dc7d4899c731c9fd4 Mon Sep 17 00:00:00 2001 From: Allan Beaufour Date: Sun, 25 Jan 2026 12:10:30 -0500 Subject: [PATCH 3/4] Export rate limit functions from package root Add exports for set_rate_limit, get_rate_limit, and get_rate_limit_status to make them accessible via flickr_api.set_rate_limit() etc. Co-Authored-By: Claude Opus 4.5 --- flickr_api/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/flickr_api/__init__.py b/flickr_api/__init__.py index 7cb7288..ebff92b 100644 --- a/flickr_api/__init__.py +++ b/flickr_api/__init__.py @@ -35,8 +35,11 @@ from .auth import set_auth_handler as set_auth_handler from .method_call import disable_cache as disable_cache from .method_call import enable_cache as enable_cache +from .method_call import get_rate_limit as get_rate_limit +from .method_call import get_rate_limit_status as get_rate_limit_status from .method_call import get_retry_config as get_retry_config from .method_call import get_timeout as get_timeout +from .method_call import set_rate_limit as set_rate_limit from .method_call import set_retry_config as set_retry_config from .method_call import set_timeout as set_timeout from .keys import set_keys as set_keys From bf0326c09c769e44936f11e5abc8efedeb6b80ae Mon Sep 17 00:00:00 2001 From: Allan Beaufour Date: Sun, 25 Jan 2026 14:12:46 -0500 Subject: [PATCH 4/4] Remove design doc Co-Authored-By: Claude Opus 4.5 --- ...26-01-25-proactive-rate-limiting-design.md | 92 ------------------- 1 file changed, 92 deletions(-) delete mode 100644 docs/plans/2026-01-25-proactive-rate-limiting-design.md diff --git a/docs/plans/2026-01-25-proactive-rate-limiting-design.md b/docs/plans/2026-01-25-proactive-rate-limiting-design.md deleted file mode 100644 index e0c3068..0000000 --- a/docs/plans/2026-01-25-proactive-rate-limiting-design.md +++ /dev/null @@ -1,92 +0,0 @@ -# Proactive Rate Limiting Design - -## Overview - -Add opt-in rate limiting to prevent exceeding Flickr's 3600 queries/hour limit. This complements the existing reactive rate limit handling (HTTP 429 retry with backoff). - -## Public API - -```python -# Enable rate limiting (3600 requests/hour = ~1 request/second) -flickr_api.set_rate_limit(requests_per_hour=3600) - -# Disable rate limiting -flickr_api.set_rate_limit(requests_per_hour=None) - -# Get current setting -flickr_api.get_rate_limit() -# Returns: {"requests_per_hour": 3600} or {"requests_per_hour": None} - -# Get status including last request time -flickr_api.get_rate_limit_status() -# Returns: { -# "enabled": True, -# "requests_per_hour": 3600, -# "interval_seconds": 1.0, -# "last_request_time": 1706198400.123 # or None if no requests yet -# } -``` - -## Design Decisions - -1. **Simple sleep-based throttling** - Sleep a calculated interval between requests. Simple and predictable. - -2. **Opt-in** - Disabled by default to preserve backward compatibility. - -3. **Requests-per-hour configuration** - Maps directly to Flickr's documented limit (3600/hour). Internally converted to interval: `interval = 3600 / requests_per_hour`. - -4. **Basic status API** - Provides enough info for debugging without over-engineering. - -## Implementation - -### Module Globals (method_call.py) - -```python -_RATE_LIMIT_REQUESTS_PER_HOUR: float | None = None # None = disabled -_RATE_LIMIT_LAST_REQUEST: float | None = None # timestamp of last request -``` - -### Throttling Function - -```python -def _maybe_wait_for_rate_limit() -> None: - """Sleep if necessary to respect rate limit.""" - global _RATE_LIMIT_LAST_REQUEST - - if _RATE_LIMIT_REQUESTS_PER_HOUR is None: - return # Rate limiting disabled - - interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR - - if _RATE_LIMIT_LAST_REQUEST is not None: - elapsed = time.time() - _RATE_LIMIT_LAST_REQUEST - if elapsed < interval: - time.sleep(interval - elapsed) - - _RATE_LIMIT_LAST_REQUEST = time.time() -``` - -### Integration Point - -Called at the start of `_make_request_with_retry()`, before making each HTTP request. - -## Files to Modify - -- `flickr_api/method_call.py` - Add rate limit globals and functions, integrate into request flow -- `flickr_api/__init__.py` - Export new functions -- `test/test_rate_limit_throttle.py` - New test file - -## Test Plan - -1. `test_rate_limit_disabled_by_default` - Verify disabled initially -2. `test_set_and_get_rate_limit` - Set/get round-trip -3. `test_disable_rate_limit` - Set to None disables -4. `test_interval_calculation` - 3600 req/hour = 1.0s, 1800 = 2.0s -5. `test_get_rate_limit_status` - Status dict structure -6. `test_no_sleep_when_disabled` - No sleep when off -7. `test_sleeps_when_interval_not_elapsed` - Correct sleep duration -8. `test_no_sleep_when_interval_elapsed` - No sleep if enough time passed -9. `test_first_request_no_sleep` - First request doesn't wait -10. `test_updates_last_request_time` - Timestamp updates correctly - -Tests will mock `time.sleep` and `time.time` for deterministic behavior.