fix: skip commits rejected by GitHub with HTTP 422

collectoss/tasks/github/facade_github/contributor_interfaceable/contributor_interface.py

@@ -1,6 +1,7 @@
 from collectoss.tasks.github.util.github_task_session import *
 import json
 import time
+import httpx
 import sqlalchemy as s
 from collectoss.application.db.models import *
 from collectoss.tasks.github.util.github_paginator import hit_api, process_dict_response
@@ -30,7 +31,7 @@ def clean_dict(d):
 @deprecated("Please use GithubDataAcess.get_resource() instead")
 def request_dict_from_endpoint(logger, session, url, timeout_wait=10):
     """Hit the endpoint specified by the url and return the json that it returns if it returns a dict.
-    
+
     NOTE: This function is being deprecated in favor of GithubDataAcess.get_resource()
     No functional change in this function body; logic preserved fully.
     This function still attempts to hit a GitHub API endpoint and return a dictionary if successful.
@@ -113,7 +114,7 @@ def create_endpoint_from_email(email):
     # Also stopped splitting per note above.
     url = 'https://api.github.com/search/users?q={}+in:email+type:user'.format(
         email)
-    
+
 
     return url
 
@@ -264,9 +265,9 @@ def update_contributor(self, cntrb, max_attempts=3):
 def fetch_username_from_email(logger, auth, commit) -> dict | None:
     """Try every distinct email found within a commit for possible username resolution.
     Add email to garbage table if can't be resolved.
-    
+
     #   \param contributor is the raw database entry
- 
+
     Args:
         logger (_type_): logging instance to use
         auth (_type_): Keymanager instance to use for auth
@@ -368,7 +369,15 @@ def get_login_with_commit_hash(logger, auth, commit_data, repo_id):
     #TODO: here.
     # Send api request
     github_data_access = GithubDataAccess(auth, logger)
-    login_json = github_data_access.get_resource(url)
+    try:
+        login_json = github_data_access.get_resource(url)
+    except httpx.HTTPStatusError as e:
+        if e.response.status_code == 422:
+            logger.warning(
+                f"GitHub rejected commit lookup for hash {commit_data['hash']} "
+                f"in repo_id={repo_id} with HTTP 422."
+            )
+        raise
 
     # TODO: Why are we returning None if 'sha' is not in response if we aren't even using it?
     if login_json is None or 'sha' not in login_json:

collectoss/tasks/github/facade_github/tasks.py

@@ -1,9 +1,12 @@
 import logging
 
-
 from collectoss.tasks.init.celery_app import celery_app as celery
 from collectoss.tasks.init.celery_app import FacadeRepoCollectionTask
-from collectoss.tasks.github.util.github_data_access import GithubDataAccess, UrlNotFoundException
+from collectoss.tasks.github.util.github_data_access import (
+    GithubDataAccess,
+    UrlNotFoundException,
+    ResourceUnprocessable,
+)
 from collectoss.tasks.github.util.github_random_key_auth import GithubRandomKeyAuth
 from collectoss.tasks.github.facade_github.core import *
 from collectoss.application.db.lib import execute_sql, get_contributor_aliases_by_email, get_unresolved_commit_emails_by_email, get_repo_by_repo_git, batch_insert_contributors, get_batch_size
@@ -62,8 +65,19 @@ def process_commit_metadata(logger, auth, contributorQueue, repo_id, platform_id
 
 
         # Try to get the login from the commit sha
-        if login == None or login == "":
-            login = get_login_with_commit_hash(logger, auth, contributor, repo_id)
+        if login is None or login == "":
+            try:
+                login = get_login_with_commit_hash(
+                    logger, auth, contributor, repo_id
+                )
+            except ResourceUnprocessable:
+                # Ideally, find another commit from this contributor.
+                # That lookup is not implemented yet, so skip this commit for now.
+                logger.warning(
+                    f"Skipping commit {contributor.get('hash')} "
+                    f"because GitHub returned HTTP 422."
+            )
+            continue
 
         if login == None or login == "":
             logger.warning("Failed to get login from commit hash")

collectoss/tasks/github/util/github_data_access.py

@@ -34,10 +34,14 @@
     def __init__(self, message="Resource returned HTTP 410 Gone. It is likely intentionally removed"):
         super().__init__(message)
 
+class ResourceUnprocessable(Exception):
+    """Exception raised when GitHub returns HTTP 422."""
+    pass
+
 class GithubDataAccess:
 
     def __init__(self, key_manager, logger: logging.Logger, feature="rest"):
-    
+
         self.logger = logger
         self.feature = feature
         self.key_client = KeyClient(f"github_{feature}", logger)
@@ -119,21 +123,21 @@
 
             yield from data
 
-        return 
-    
+        return
+
     def get_resource_page_count(self, url):
 
         response = self.make_request_with_retries(url, method="HEAD")
 
         if 'last' not in response.links.keys():
             self.logger.warning(f"Github response without links. Headers: {response.headers}.")
             return 1
-        
+
         try:
             last_page_url = response.links['last']['url']
 
             parsed_url = urlparse(last_page_url)
-        
+
             return int(parse_qs(parsed_url.query)['page'][0])
         except (KeyError, ValueError):
             raise Exception(f"Unable to parse 'last' url from response: {response.links['last']}")
@@ -142,8 +146,8 @@
 
         response = self.make_request_with_retries(url)
 
-        return response.json()  
-    
+        return response.json()
+
     # TODO: Handle timeout exceptions better
     def make_request(self, url, method="GET", timeout=100):
 
@@ -161,13 +165,13 @@
                 self.logger.warning(f"Github rate limit exceeded. Key: {mask_key(self.key)}. Response: {response.text}")
                 raise RatelimitException(response, self.expired_keys_for_request)
 
-            # There are cases with PR files, PR commits, and messages where the parent object is removed after 
-            # It is collected, leading the the associated URL for those objects to return a 404. 
-            # This is not an issue that is really an Exception. It is more of a nominal signal. 
-            
+            # There are cases with PR files, PR commits, and messages where the parent object is removed after
+            # It is collected, leading the the associated URL for those objects to return a 404.
+            # This is not an issue that is really an Exception. It is more of a nominal signal.
+
             if response.status_code == 404:
                 raise UrlNotFoundException(f"Could not find {url}")
-            
+
             if response.status_code == 401:
                 resp_content = response.json()
                 response_msg = resp_content.get("message")
@@ -178,14 +182,18 @@
                     raise NotAuthorizedException(f"Could not authorize with the github api because key was invalid: {mask_key(self.key)}")
                 else:
                     self.logger.warning(f"Received a 401 response from github unrelated to bad credentials with message {response_msg}")
-            
+
             if response.status_code == 410:
                 response_msg = response.json().get("message")
                 if response_msg is not None:
                     raise ResourceGoneException(response_msg)
                 else:
                     raise ResourceGoneException()
-
+            if response.status_code == 422:
+                raise ResourceUnprocessable(
+                    f"GitHub returned HTTP 422 for {url}"
+                )
+
             response.raise_for_status()
 
             try:
@@ -197,7 +205,7 @@
 
 
             return response
-        
+
     def make_request_with_retries(self, url, method="GET", timeout=100):
         """ What method does?
             1. Catches RetryError and rethrows a nicely formatted OutOfRetriesException that includes that last exception thrown
@@ -212,17 +220,24 @@
             # this suggests we should retry 401 exceptions at least once
             if isinstance(last_exception, NotAuthorizedException):
                 self.expired_keys_for_request = []
-                self.__handle_github_not_authorized_response()           
+                self.__handle_github_not_authorized_response()
             raise last_exception
 
     def _decide_retry_policy(exception: Exception) -> bool:
         """Defines whether or not to retry a failed request based on the exception thrown
 
         Returns:
             bool: Boolean describing whether or not the request should be retried
         """
-        return not isinstance(exception, (UrlNotFoundException, ResourceGoneException))
-
+        return not isinstance(
+            exception,
+            (
+                UrlNotFoundException,
+                ResourceGoneException,
+                ResourceUnprocessable,
+            ),
+     )
+
     @retry(stop=stop_after_attempt(10), wait=wait_fixed(5), retry=retry_if_exception(_decide_retry_policy))
     def __make_request_with_retries(self, url, method="GET", timeout=100):
         """ What method does?
@@ -244,7 +259,7 @@
 
         self.key = self.key_client.invalidate(self.key)
 
-        
+
     def __handle_github_ratelimit_response(self, response):
 
         headers = response.headers
@@ -260,11 +275,11 @@
             current_epoch = int(time.time())
             epoch_when_key_resets = int(headers["X-RateLimit-Reset"])
             key_reset_time =  epoch_when_key_resets - current_epoch
-            
+
             if key_reset_time < 0:
                 self.logger.error(f"Key reset time was less than 0 setting it to 0.\nThe current epoch is {current_epoch} and the epoch that the key resets at is {epoch_when_key_resets}")
                 key_reset_time = 0
-                
+
             self.logger.info(f"\n\n\nAPI rate limit exceeded. Key resets in {key_reset_time} seconds. Informing key manager that key is expired")
             self.key = self.key_client.expire(self.key, epoch_when_key_resets)
 

tests/test_classes/test_github_data_access.py

@@ -2,8 +2,13 @@
 import pytest
 from unittest.mock import Mock, patch
 
-from collectoss.tasks.github.util.github_data_access import GithubDataAccess
-
+from collectoss.tasks.github.util.github_data_access import (
+    GithubDataAccess,
+    ResourceUnprocessable,
+)
+from collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface import (
+    get_login_with_commit_hash,
+)
 
 @pytest.fixture
 def mock_logger():
@@ -55,3 +60,35 @@ def test_path_with_existing_query_params(self, gda):
         assert "q=python" in result
         assert "per_page=10" in result
         assert result.startswith("https://api.github.com/search/repositories")
+
+@pytest.mark.unit
+class TestResponseErrors:
+
+    def test_422_response_is_not_retried(self):
+        exception = ResourceUnprocessable(
+            "GitHub returned HTTP 422"
+        )
+
+        assert not GithubDataAccess._decide_retry_policy(exception)
+def test_422_commit_lookup_raises_resource_unprocessable():
+    logger = Mock()
+    auth = Mock()
+    commit_data = {"hash": "1f5270431bc8556cf6dfee1905e07abcc45e20d2"}
+
+    error = ResourceUnprocessable(
+        "GitHub returned HTTP 422"
+    )
+
+    with patch(
+        "collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface.create_endpoint_from_commit_sha",
+        return_value="https://api.github.com/repos/Azure/Avere/commits/test",
+    ), patch(
+        "collectoss.tasks.github.facade_github.contributor_interfaceable.contributor_interface.GithubDataAccess"
+    ) as mock_gda:
+        mock_gda.return_value.get_resource.side_effect = error
+
+        with pytest.raises(ResourceUnprocessable):
+            get_login_with_commit_hash(
+                logger, auth, commit_data, repo_id=1
+            )
+