-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ASOC-2886] Add retries when sending graphql requests #38
Conversation
finite_state_sdk/__init__.py
Outdated
@@ -1968,6 +1974,7 @@ def search_sbom(token, organization_context, name=None, version=None, asset_vers | |||
return records | |||
|
|||
|
|||
@retry(stop=stop_after_attempt(10), wait=wait_fixed(10), retry=retry_if_exception(is_not_breakout_exception)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is too many retries, maybe 3? 5? how long does the test take to run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to 5
# Call the function and expect a RetryError | ||
with pytest.raises(tenacity.RetryError) as excinfo: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this test takes too long to run (cause of all the retries) you can add a send_graphql_query.retry.wait = wait_fixed(0)
in the test itself to override the wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only takes couple minutes.
# Assertion | ||
assert str(excinfo.value) == "Error: 500 - Internal Server Error" | ||
# Check the inner exception message | ||
inner_exception = excinfo.value.last_attempt.exception() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also assert the number of calls to post
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
ASOC-2886 - Fixed an issue with the SDK that was either not working or failing intermittently. Added retries (up to 10 times) for queries, but skips retries for mutations.