From 7089150fe655e7701baa97a6c67e09c5e11d6675 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 18 Nov 2024 12:49:24 +0100 Subject: [PATCH 1/4] Handle github api rate limits --- tmt/utils/__init__.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index c1b258ba8d..657a0accb4 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -4358,6 +4358,33 @@ def increment( raise GeneralError(message) from error + # Handle GitHub-specific responses + # https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#exceeding-the-rate-limit + response = kwargs.get('response') + if response is not None: + # Check if it's a GitHub API response by looking for specific headers + headers = getattr(response, 'headers', {}) + if 'X-GitHub-Request-Id' in headers and response.status in (403, 429): + # Primary rate limit exceeded + if ('X-RateLimit-Remaining' in headers and + int(headers['X-RateLimit-Remaining']) == 0): + reset_time = int(headers['X-RateLimit-Reset']) + # Wait until the rate limit resets + wait_time = reset_time - int(time.time()) + if wait_time > 0: + time.sleep(wait_time) + return super().increment(*args, **kwargs) + + # Secondary rate limit - respect Retry-After if provided + if 'Retry-After' in headers: + retry_after = int(headers['Retry-After']) + time.sleep(retry_after) + return super().increment(*args, **kwargs) + + # No Retry-After header for secondary limit, use exponential backoff + # Let the parent Retry class handle the backoff timing + return super().increment(*args, **kwargs) + return super().increment(*args, **kwargs) From a7c121ecb180f1f2754fe64951a15c3848d63ac2 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 18 Nov 2024 13:00:55 +0100 Subject: [PATCH 2/4] Add test for github rate limits retry strategy --- tests/unit/test_utils.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 22655866a0..7b2541df22 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1854,3 +1854,25 @@ def test_render_command_report_minimal(): label='foo' )) == """## foo """ + + +def test_retry_strategy_github_rate_limits(monkeypatch): + """ Test GitHub rate limit handling in RetryStrategy """ + strategy = tmt.utils.RetryStrategy() + mock_time = unittest.mock.MagicMock() + monkeypatch.setattr('time.time', mock_time) + mock_time.return_value = 1704114000 # Example: 2024-01-01 13:00:00 UTC + mock_sleep = unittest.mock.MagicMock() + monkeypatch.setattr('time.sleep', mock_sleep) + + # Test primary rate limit + mock_response = unittest.mock.MagicMock() + mock_response.status = 403 + mock_response.headers = { + 'X-GitHub-Request-Id': 'ABC123', + 'X-RateLimit-Remaining': '0', + 'X-RateLimit-Reset': '1704114100' # 2024-01-01 13:01:40 UTC (100 seconds later) + } + + strategy.increment(error=None, response=mock_response) + mock_sleep.assert_called_once_with(100) # Should wait 100 seconds until reset time From ed66375b69a8e72be211fc208d36891f69242b6c Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 18 Nov 2024 15:43:46 +0100 Subject: [PATCH 3/4] fixup! Handle github api rate limits --- tmt/utils/__init__.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index 657a0accb4..072db75b02 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -4360,11 +4360,12 @@ def increment( # Handle GitHub-specific responses # https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#exceeding-the-rate-limit - response = kwargs.get('response') - if response is not None: - # Check if it's a GitHub API response by looking for specific headers - headers = getattr(response, 'headers', {}) - if 'X-GitHub-Request-Id' in headers and response.status in (403, 429): + response = cast(requests.Response, kwargs.get('response')) + if response is not None and 'X-GitHub-Request-Id' in response.headers: + headers = response.headers + + # Handle rate limiting + if response.status_code in (403, 429): # Primary rate limit exceeded if ('X-RateLimit-Remaining' in headers and int(headers['X-RateLimit-Remaining']) == 0): @@ -4381,10 +4382,6 @@ def increment( time.sleep(retry_after) return super().increment(*args, **kwargs) - # No Retry-After header for secondary limit, use exponential backoff - # Let the parent Retry class handle the backoff timing - return super().increment(*args, **kwargs) - return super().increment(*args, **kwargs) From e7b58f3f95d5bad3feada0d50c6b39c736f71c63 Mon Sep 17 00:00:00 2001 From: Martin Hoyer Date: Mon, 18 Nov 2024 15:45:28 +0100 Subject: [PATCH 4/4] fixup! Add test for github rate limits retry strategy --- tests/unit/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 7b2541df22..258f74e567 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1867,7 +1867,7 @@ def test_retry_strategy_github_rate_limits(monkeypatch): # Test primary rate limit mock_response = unittest.mock.MagicMock() - mock_response.status = 403 + mock_response.status_code = 403 mock_response.headers = { 'X-GitHub-Request-Id': 'ABC123', 'X-RateLimit-Remaining': '0',