Fix redhat import failure #418
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduced and used a helper function for retries on 5xx errors. This is important and some servers like bugzilla.redhat.com return 502 Proxy Error which was the cause of aboutcode-org#398 A ticket has been raised in RedHat here https://redhat.service-now.com/help?id=rh_ticket&table=sc_req_item&sys_id=278239541b1ba010477e43fccd4bcb4a Signed-off-by: Hritik Vijay <[email protected]>
This is mentioned in the NOTE of "2.1 List all CVRFs" of https://access.redhat.com/documentation/en-us/red_hat_security_data_api/1.0/html/red_hat_security_data_api/cvrf Such a case would lead to a crash before this commit. Eg: https://access.redhat.com/hydra/rest/securitydata/cvrf/RHSA-2005:835.json No cvrfdoc would be found in the statement value = rhsa_data["cvrfdoc"]["aggregate_severity"] Signed-off-by: Hritik Vijay <[email protected]>
This finally fixes aboutcode-org#398 Signed-off-by: Hritik Vijay <[email protected]>
Previous commits replace the usage of requests.get() altogether with a custom requests_session which provides better 5xx error handling. It is now required to mock that object in this test. IMHO it would make more sense to update this test altogether to use the real endpoints against some real data. Signed-off-by: Hritik Vijay <[email protected]>
sbs2001
approved these changes
Apr 2, 2021
| @@ -79,3 +80,21 @@ def create_etag(data_src, url, etag_key): | |||
|
|
|||
|
|
|||
| is_cve = re.compile(r"CVE-\d+-\d+", re.IGNORECASE).match | |||
|
|
|||
|
|
|||
| def requests_with_5xx_retry(max_retries=5, backoff_factor=0.5): | |||
There was a problem hiding this comment.
You said there were 104 errors, how are they addressed ?
There was a problem hiding this comment.
104 came after retrying with short delays for multiple times. I increased the backoff_factor to 1 in the redhat importer.
|
@sbs2001 Perhaps we would be able to revert back after redhat fixes the problem on their side. Until then, this is what we got. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #398.
This PR is mainly comprises of following two commits.
requests_with_5xx_retry: Retry on 5xx errors
Introduced and used a helper function for retries on 5xx errors. This is
important and some servers like bugzilla.redhat.com return 502 Proxy Error
which was the cause of #398
A ticket has been raised in RedHat here https://redhat.service-now.com/help?id=rh_ticket&table=sc_req_item&sys_id=278239541b1ba010477e43fccd4bcb4a
Here is the response from [email protected]
Not all RHSA errata have a CVRF document
This is mentioned in the NOTE of "2.1 List all CVRFs" of
https://access.redhat.com/documentation/en-us/red_hat_security_data_api/1.0/html/red_hat_security_data_api/cvrf
Such a case would lead to a crash before this commit.
Eg: https://access.redhat.com/hydra/rest/securitydata/cvrf/RHSA-2005:835.json
No cvrfdoc would be found in the statement
value = rhsa_data["cvrfdoc"]["aggregate_severity"]