Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 54 additions & 20 deletions gittensor/utils/github_api_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,29 +248,53 @@ def get_github_account_age_days(token: str) -> Optional[int]:

def get_pull_request_file_changes(repository: str, pr_number: int, token: str) -> Optional[List[FileChange]]:
"""
Get the diff for a specific PR by repository name and PR number
Get the diff for a specific PR by repository name and PR number.

Uses retry logic with exponential backoff for transient failures.

Args:
repository (str): Repository in format 'owner/repo'
pr_number (int): PR number
token (str): Github pat
Returns:
List[FileChanges]: List object with file changes or None if error
"""
max_attempts = 3
headers = make_headers(token)

try:
response = requests.get(
f'{BASE_GITHUB_API_URL}/repos/{repository}/pulls/{pr_number}/files', headers=headers, timeout=15
)
if response.status_code == 200:
file_diffs = response.json()
return [FileChange.from_github_response(pr_number, repository, file_diff) for file_diff in file_diffs]
last_error = None
for attempt in range(max_attempts):
try:
response = requests.get(
f'{BASE_GITHUB_API_URL}/repos/{repository}/pulls/{pr_number}/files', headers=headers, timeout=15
)
if response.status_code == 200:
file_diffs = response.json()
return [FileChange.from_github_response(pr_number, repository, file_diff) for file_diff in file_diffs]

return []
last_error = f'status {response.status_code}'
if attempt < max_attempts:
backoff_delay = min(5 * (2**attempt), 30)
bt.logging.warning(
f'File changes request for PR #{pr_number} in {repository} failed with status {response.status_code} '
f'(attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...'
)
time.sleep(backoff_delay)

except Exception as e:
bt.logging.error(f'Error getting file changes for PR #{pr_number} in {repository}: {e}')
return []
except requests.exceptions.RequestException as e:
last_error = str(e)
if attempt < max_attempts:
backoff_delay = min(5 * (2**attempt), 30)
bt.logging.warning(
f'File changes request error for PR #{pr_number} in {repository} '
f'(attempt {attempt + 1}/{max_attempts}): {e}, retrying in {backoff_delay}s...'
)
time.sleep(backoff_delay)

bt.logging.error(
f'File changes request for PR #{pr_number} in {repository} failed after {max_attempts} attempts: {last_error}'
)
return []


def execute_graphql_query(
Expand Down Expand Up @@ -354,13 +378,14 @@ def get_github_graphql_query(

max_attempts = 8
headers = {'Authorization': f'Bearer {token}', 'Content-Type': 'application/json'}
variables = {
'userId': global_user_id,
'limit': min(100, max_prs - merged_pr_count),
'cursor': cursor,
}
limit = min(100, max_prs - merged_pr_count)

for attempt in range(max_attempts):
variables = {
'userId': global_user_id,
'limit': limit,
'cursor': cursor,
}
try:
response = requests.post(
f'{BASE_GITHUB_API_URL}/graphql',
Expand All @@ -375,9 +400,18 @@ def get_github_graphql_query(
# error - log and retry
if attempt < (max_attempts - 1):
backoff_delay = min(5 * (2**attempt), 30) # cap at 30s
bt.logging.warning(
f'GraphQL request for PRs failed with status {response.status_code} (attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...'
)
# Reduce page size on server-side errors (query may be too expensive)
if response.status_code in (502, 503, 504):
limit = max(limit // 2, 10)
bt.logging.warning(
f'GraphQL request for PRs failed with status {response.status_code} '
f'(attempt {attempt + 1}/{max_attempts}), page size set to {limit}, retrying in {backoff_delay}s...'
)
else:
bt.logging.warning(
f'GraphQL request for PRs failed with status {response.status_code} '
f'(attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...'
)
time.sleep(backoff_delay)
else:
bt.logging.error(
Expand Down
200 changes: 200 additions & 0 deletions tests/utils/test_github_api_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
get_github_graphql_query = github_api_tools.get_github_graphql_query
get_github_id = github_api_tools.get_github_id
get_github_account_age_days = github_api_tools.get_github_account_age_days
get_pull_request_file_changes = github_api_tools.get_pull_request_file_changes


# ============================================================================
Expand Down Expand Up @@ -101,6 +102,94 @@ def test_retry_on_502_then_success(self, mock_logging, mock_sleep, mock_post, gr
# Verify exponential backoff: 5s, 10s
mock_sleep.assert_has_calls([call(5), call(10)])

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_502_halves_page_size_on_retry(self, mock_logging, mock_sleep, mock_post, graphql_params):
"""Test that 502 errors cause the page size (limit) to be halved on each retry."""
mock_response_502 = Mock(status_code=502, text='502 Bad Gateway')
mock_response_200 = Mock(status_code=200)

mock_post.side_effect = [mock_response_502, mock_response_502, mock_response_200]

result = get_github_graphql_query(**graphql_params)

assert result.status_code == 200
# Initial limit=100, halved to 50 after first 502, halved to 25 after second 502
limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list]
assert limits == [100, 50, 25]

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_page_size_floors_at_10(self, mock_logging, mock_sleep, mock_post, graphql_params):
"""Test that page size never drops below 10 even after many 502s."""
mock_response_502 = Mock(status_code=502, text='502 Bad Gateway')
mock_post.return_value = mock_response_502

get_github_graphql_query(**graphql_params)

# 100 -> 50 -> 25 -> 12 -> 10 -> 10 -> 10 -> 10
limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list]
assert limits == [100, 50, 25, 12, 10, 10, 10, 10]

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_non_5xx_does_not_reduce_page_size(self, mock_logging, mock_sleep, mock_post, graphql_params):
"""Test that non-5xx errors (e.g. 401) do not reduce page size."""
mock_response_401 = Mock(status_code=401, text='Unauthorized')
mock_response_200 = Mock(status_code=200)

mock_post.side_effect = [mock_response_401, mock_response_401, mock_response_200]

result = get_github_graphql_query(**graphql_params)

assert result.status_code == 200
limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list]
assert limits == [100, 100, 100], 'Page size should stay at 100 for non-5xx errors'

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_503_and_504_also_reduce_page_size(self, mock_logging, mock_sleep, mock_post, graphql_params):
"""Test that 503 and 504 errors also trigger page size reduction."""
mock_response_503 = Mock(status_code=503, text='Service Unavailable')
mock_response_504 = Mock(status_code=504, text='Gateway Timeout')
mock_response_200 = Mock(status_code=200)

mock_post.side_effect = [mock_response_503, mock_response_504, mock_response_200]

result = get_github_graphql_query(**graphql_params)

assert result.status_code == 200
limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list]
assert limits == [100, 50, 25]

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_small_initial_limit_not_reduced_below_10(self, mock_logging, mock_sleep, mock_post):
"""Test that a small initial limit (e.g. 15) floors correctly at 10."""
mock_response_502 = Mock(status_code=502, text='502 Bad Gateway')
mock_response_200 = Mock(status_code=200)

mock_post.side_effect = [mock_response_502, mock_response_502, mock_response_200]

params = {
'token': 'fake_github_token',
'global_user_id': 'MDQ6VXNlcjEyMzQ1',
'merged_pr_count': 85,
'max_prs': 100,
'cursor': None,
}
result = get_github_graphql_query(**params)

assert result.status_code == 200
# Initial limit = min(100, 100-85) = 15, halved to 10, stays at 10
limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list]
assert limits == [15, 10, 10]

@patch('gittensor.utils.github_api_tools.requests.post')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
Expand Down Expand Up @@ -292,5 +381,116 @@ def test_get_github_account_age_retry_logic(self, mock_sleep, mock_get, clear_gi
assert mock_get.call_count == 2


# ============================================================================
# File Changes Retry Logic Tests
# ============================================================================


class TestFileChangesRetryLogic:
"""Test suite for retry logic in get_pull_request_file_changes."""

@patch('gittensor.utils.github_api_tools.requests.get')
def test_successful_request_no_retry(self, mock_get):
"""Test that a successful request returns file changes without retrying."""
mock_response = Mock(status_code=200)
mock_response.json.return_value = [
{
'filename': 'test.py',
'status': 'modified',
'changes': 2,
'additions': 1,
'deletions': 1,
'patch': '@@ -1 +1 @@\n-old\n+new',
},
]
mock_get.return_value = mock_response

result = get_pull_request_file_changes('owner/repo', 1, 'fake_token')

assert mock_get.call_count == 1
assert result is not None
assert len(result) == 1

@patch('gittensor.utils.github_api_tools.requests.get')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_retry_on_502_then_success(self, mock_logging, mock_sleep, mock_get):
"""Test that 502 triggers retry and succeeds on second attempt."""
mock_502 = Mock(status_code=502, text='Bad Gateway')
mock_200 = Mock(status_code=200)
mock_200.json.return_value = [
{'filename': 'test.py', 'status': 'modified', 'changes': 1, 'additions': 1, 'deletions': 0, 'patch': ''},
]

mock_get.side_effect = [mock_502, mock_200]

result = get_pull_request_file_changes('owner/repo', 1, 'fake_token')

assert mock_get.call_count == 2
assert mock_sleep.call_count == 1
assert len(result) == 1

@patch('gittensor.utils.github_api_tools.requests.get')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_gives_up_after_three_attempts(self, mock_logging, mock_sleep, mock_get):
"""Test that function gives up after 3 failed attempts and returns empty list."""
mock_500 = Mock(status_code=500, text='Internal Server Error')
mock_get.return_value = mock_500

result = get_pull_request_file_changes('owner/repo', 1, 'fake_token')

assert mock_get.call_count == 3
assert mock_sleep.call_count == 3
assert result == []
mock_logging.error.assert_called()

@patch('gittensor.utils.github_api_tools.requests.get')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_retry_on_connection_error_then_success(self, mock_logging, mock_sleep, mock_get):
"""Test that connection errors trigger retry."""
import requests

mock_200 = Mock(status_code=200)
mock_200.json.return_value = []

mock_get.side_effect = [requests.exceptions.ConnectionError('refused'), mock_200]

result = get_pull_request_file_changes('owner/repo', 1, 'fake_token')

assert mock_get.call_count == 2
assert mock_sleep.call_count == 1
assert result == []

@patch('gittensor.utils.github_api_tools.requests.get')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_gives_up_after_three_connection_errors(self, mock_logging, mock_sleep, mock_get):
"""Test that function gives up after 3 connection errors."""
import requests

mock_get.side_effect = requests.exceptions.ConnectionError('refused')

result = get_pull_request_file_changes('owner/repo', 1, 'fake_token')

assert mock_get.call_count == 3
assert result == []
mock_logging.error.assert_called()

@patch('gittensor.utils.github_api_tools.requests.get')
@patch('gittensor.utils.github_api_tools.time.sleep')
@patch('gittensor.utils.github_api_tools.bt.logging')
def test_exponential_backoff_timing(self, mock_logging, mock_sleep, mock_get):
"""Test that backoff delays are 5s, 10s for 3 attempts."""
mock_500 = Mock(status_code=500, text='Internal Server Error')
mock_get.return_value = mock_500

get_pull_request_file_changes('owner/repo', 1, 'fake_token')

mock_sleep.assert_has_calls([call(5), call(10), call(20)])
assert mock_sleep.call_count == 3


if __name__ == '__main__':
pytest.main([__file__, '-v'])