GitHub intel updates to error handling #1303
Conversation
serge-wq
changed the title
| if 'data' not in resp: | ||
| logger.warning( | ||
| f'Got no "data" attribute in response: {resp}. ' | ||
| f'Stopping requests for organization: {organization} and ' | ||
| f'resource_type: {resource_type}', | ||
| ) | ||
| has_next_page = False | ||
| continue |
There was a problem hiding this comment.
is this a known issue that could happen? It's like a key in a dictionary that we expect but doesn't exist; and that can happen in many other places. Wonder if we know more about this particular case to suspect it might happen here.
There was a problem hiding this comment.
data is the root element from GH GraphQL API.
Unfortunately, there is no official documentation explicitly describing the response 🙃 .
But, in case it doesn't exists, the code cannot get any data to ingest.
There was a problem hiding this comment.
As @heryxpc said, GraphQL response usually contain data as the root attribute in the response which contains the actual requested data. This is a convention I've seen across GraphQL, not only for GitHub. Another thing that I've seen across GraphQL APIs is that if there is no data available, the attributes are returned as undefined, null or not even present. That's why this check here ill help us know what happened.
There was a problem hiding this comment.
Also, this issue is what has been breaking the GitHub crons in our infra. As it's unclear in which page it happens, I'm just breaking out the loop gracefully and making sure we at least got the org_data.
|
|
||
| org_data = {'url': resp['data']['organization']['url'], 'login': resp['data']['organization']['login']} |
There was a problem hiding this comment.
I assume the change here is that: in the past, this information is retrieved in the last.
We change this to collect this info on every page if not already collected.
There was a problem hiding this comment.
org_data is mandatory to perform the full ingestion of Github info to Cartography: https://github.com/lyft/cartography/blob/fa1be115c80e83cefe0e31be5d738b5cda9a0887/cartography/intel/github/users.py#L116
I believe below code ensures that it exists on any page, otherwise it breaks the full process before starting ingestion.
## Summary We've found constant errors in the GitHub module when the response data does not have the expected schema. These updates, while not always prevent the crash, they will provide more insight into what happened.
Summary
We've found constant errors in the GitHub module when the response data does not have the expected schema. These updates, while not always prevent the crash, they will provide more insight into what happened.