Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add retries for FirehoseEndpoint::load_blocks_by_numbers #5838

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

incrypto32
Copy link
Member

No description provided.

@incrypto32 incrypto32 force-pushed the krishna/sc-firehose-retries branch 5 times, most recently from 544b50c to ddb28a2 Compare February 21, 2025 12:02
Copy link
Contributor

@zorancv zorancv left a comment

Choose a reason for hiding this comment

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

Besides the uncertainty of a need for pining, the rest looks fine.

.run(move || {
let e = endpoint_for_retry.cheap_clone();
let l = logger_for_retry.clone();
Box::pin(async move { e.get_block_by_number::<M>(number, &l).await })
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a silly question, but why is pinning needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing, out. It was a leftover from when i moved the method elsewhere. Updated

@incrypto32 incrypto32 force-pushed the krishna/sc-firehose-retries branch from ddb28a2 to fe5db3c Compare February 21, 2025 15:39
@incrypto32 incrypto32 merged commit b27502c into master Feb 21, 2025
6 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants