Skip to content

feat(era): Add EraStream that downloads all era files #15613

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

Merged
merged 20 commits into from
Apr 14, 2025
Merged

Conversation

RomanHodulak
Copy link
Collaborator

@RomanHodulak RomanHodulak commented Apr 8, 2025

Closes #15496

Adds EraStream to stream filenames of downloaded files in a given folder.

Some rests:

  • Add checksum validation to detect files integrity
    • When continuing download from an interrupted run, the latest file might have not been downloaded fully
    • Any other data integrity corruption
  • ☑️ Tests could be rewritten so that they don't depend on real network
    • Gives more control and possibility to cover more test cases more easily and predictably
    • Increases tests performance
    • Makes tests not depent on availability of external online services
  • Add sleep inbetween some futures
    • When folder is full and there are no ongoing downloads, the polls to recount files are happening very quickly
    • Adding a sleep future might avoid a bunch of unneeded polls

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Apr 8, 2025
@github-actions github-actions bot added A-db Related to the database C-enhancement New feature or request labels Apr 8, 2025
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

great start, I think most of the functionality is already there.

I have some suggestions for structuring this, I think converting this into an actual stream type will be more flexible later, for example if we need to then route the downloaded files somewhere else we can easily add a channel on top

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Apr 9, 2025
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is looking great, love the tests.

have some (pedantic) nits.

I think we should also consider sprinkling a few trace! calls here and there, e.g. when we started/finished downloading a file

@RomanHodulak RomanHodulak changed the title WIP: feat(era): Add EraClient that downloads all era files feat(era): Add EraStream that downloads all era files Apr 11, 2025
@RomanHodulak RomanHodulak force-pushed the era-downloader branch 2 times, most recently from 3caf789 to dfda1d1 Compare April 11, 2025 15:37
@RomanHodulak RomanHodulak marked this pull request as ready for review April 11, 2025 15:39
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice, left some suggestions, but otherwise this lgtm

The polls are happening very quickly even when nothing is happening

this could be due to the waker, which, from what I can see, can be removed entirely

Add checksum validation to detect files integrity
When continuing download from an interrupted run, the latest file might have not been downloaded fully
Any other data integrity corruption

all of these additional features we def want but can be added as followups

Previous behavior would end the stream and throw away the error
@RomanHodulak RomanHodulak added this pull request to the merge queue Apr 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 14, 2025
@RomanHodulak RomanHodulak added this pull request to the merge queue Apr 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 14, 2025
@RomanHodulak RomanHodulak added this pull request to the merge queue Apr 14, 2025
@RomanHodulak RomanHodulak removed this pull request from the merge queue due to a manual request Apr 14, 2025
@RomanHodulak RomanHodulak enabled auto-merge April 14, 2025 13:57
@RomanHodulak RomanHodulak added this pull request to the merge queue Apr 14, 2025
Merged via the queue into main with commit 1e0b433 Apr 14, 2025
45 checks passed
@RomanHodulak RomanHodulak deleted the era-downloader branch April 14, 2025 14:21
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Related to the database C-enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add downloader for era stores
2 participants