Skip to content
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

test(ICRC_Rosetta): FI-1699: Bump the retry limit #4446

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbjorkqvist
Copy link
Member

Bump the retry limit from 10s to 30s while waiting for transactions to be added to the blockchain.

@github-actions github-actions bot added the test label Mar 20, 2025
@mbjorkqvist
Copy link
Member Author

The test has been flaky on CI, and it was also reproducible in the devenv container e.g., here. After bumping the retry limit, in this test run with 1000 repetitions, no failures were due to this issue.

@@ -179,7 +179,8 @@ impl RosettaClient {
let request = SearchTransactionsRequest::builder(network_identifier.clone())
.with_transaction_identifier(submit_response.transaction_identifier.clone())
.build();
while tries < 10 {
const MAX_RETRIES: u64 = 30;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a change in the production code, not test code. Another option could be to allow specifying this value as a configuration option?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a downside to increasing the limit in prod?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question!

Another thought I had was to split the functionality of make_submit_and_wait_for_transaction into make_submit and wait_for_transaction methods, and then the caller could decide how often and for how long they would call wait_for_transaction if the transaction doesn't get added to the blockchain. The methods called internally in make_submit_and_wait_for_transaction seem to all be public, so a caller could already now mimic the functionality - we could also consider doing that in the tests.

Maybe @fsodre has some views?

@mbjorkqvist mbjorkqvist marked this pull request as ready for review March 20, 2025 15:09
@mbjorkqvist mbjorkqvist requested a review from a team as a code owner March 20, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants