diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index b24cdd1b..e081758c 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -178,8 +178,8 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. # This prevents predictable paths and ensures isolation between concurrent webhook handlers self.clone_repo_dir: str = tempfile.mkdtemp(prefix=f"github-webhook-{self.repository_name}-") self._repo_cloned: bool = False # Track if repository has been cloned - # Initialize auto-verified users from API users - self.add_api_users_to_auto_verified_and_merged_users() + # Note: auto-verified users from API users are initialized in process() + # because the method is async and requires asyncio.to_thread() for blocking calls self.current_pull_request_supported_retest = self._current_pull_request_supported_retest self.issue_url_for_welcome_msg: str = ( @@ -408,6 +408,9 @@ def redact_output(value: str) -> str: raise RuntimeError(f"Repository clone failed: {ex}") from ex async def process(self) -> Any: + # Initialize auto-verified users from API users (async operation) + await self.add_api_users_to_auto_verified_and_merged_users() + event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" # Start webhook routing context step @@ -593,16 +596,40 @@ async def process(self) -> Any: await self._update_context_metrics() return None - def add_api_users_to_auto_verified_and_merged_users(self) -> None: + async def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) - for _api, _ in apis_and_tokens: - if _api.rate_limiting[-1] == 60: + + async def check_token(api: github.Github, token: str) -> str | None: + """Check a single API token and return the user login if valid, None otherwise.""" + token_suffix = f"...{token[-4:]}" if token else "unknown" + try: + rate_limit_remaining = await asyncio.to_thread(lambda: api.rate_limiting[-1]) + except GithubException as ex: self.logger.warning( - f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token, skipping" + f"{self.log_prefix} Failed to get API rate limit for token ending in '{token_suffix}', " + f"skipping. {ex}" ) - continue + return None + + if rate_limit_remaining == 60: + self.logger.warning( + f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " + f"(token ending in '{token_suffix}'), skipping" + ) + return None + + try: + _api_user = await asyncio.to_thread(lambda: api.get_user().login) + except GithubException as ex: + self.logger.exception( + f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" + ) + return None + + return _api_user - self.auto_verified_and_merged_users.append(_api.get_user().login) + results = await asyncio.gather(*[check_token(api, token) for api, token in apis_and_tokens]) + self.auto_verified_and_merged_users.extend(user for user in results if user is not None) def prepare_log_prefix(self, pull_request: PullRequest | None = None) -> str: return prepare_log_prefix( diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index 5e0d58a9..3f8655a2 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -621,13 +621,14 @@ def test_init_failed_repository_objects( assert gh.repository is None assert not hasattr(gh, "repository_by_github_app") + @pytest.mark.asyncio @patch("webhook_server.libs.github_api.Config") @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") @patch("webhook_server.libs.github_api.get_github_repo_api") @patch("webhook_server.libs.github_api.get_repository_github_app_api") @patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") @patch("webhook_server.libs.github_api.get_apis_and_tokes_from_config") - def test_add_api_users_to_auto_verified_and_merged_users( + async def test_add_api_users_to_auto_verified_and_merged_users( self, mock_get_apis, mock_color, @@ -639,7 +640,7 @@ def test_add_api_users_to_auto_verified_and_merged_users( minimal_headers, logger, ) -> None: - """Test the add_api_users_to_auto_verified_and_merged_users property.""" + """Test the add_api_users_to_auto_verified_and_merged_users method.""" mock_config.return_value.repository = True mock_config.return_value.repository_local_data.return_value = {} mock_get_api.return_value = (Mock(), "token", "apiuser") @@ -687,9 +688,9 @@ def get_value_side_effect(value, *args, **kwargs): mock_user = Mock() mock_user.login = "test-user" mock_api.get_user.return_value = mock_user - mock_get_apis.return_value = [(mock_api, "token")] + mock_get_apis.return_value = [(mock_api, TEST_GITHUB_TOKEN)] gh = GithubWebhook(minimal_hook_data, minimal_headers, logger) - _ = gh.add_api_users_to_auto_verified_and_merged_users + await gh.add_api_users_to_auto_verified_and_merged_users() assert "test-user" in gh.auto_verified_and_merged_users @patch("webhook_server.libs.github_api.get_apis_and_tokes_from_config")