-
Notifications
You must be signed in to change notification settings - Fork 127
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
blocks-import-script: add --csv-output flag #2982
Conversation
652b6c3
to
326666c
Compare
326666c
to
bd87929
Compare
print(f"\nblocks: {total_blocks}, baseline: {prettySecs(time_xt)}, contender: {prettySecs(time_yt)}") | ||
print(f"Time (total): {prettySecs(timet)}, {(timet/time_xt):.2%}") |
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 we might want to include these in the CSV file as well, possibly as extra columns.
It will cause data duplication, but it's better than adding weird rows that don't match column headers.
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.
oh yes, I forgot we had discussed this today.
I'll update.
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.
bd87929
to
fcc21ea
Compare
Summary
This PR adds
--csv-output
support to blocks import script.Github renders CSVs as tables and this addition would be useful in
nimbus-eth1-benchmark
repo to make this comparison easy to render.To see how github renders CSVs : https://github.com/status-im/nimbus-eth1-benchmarks/blob/master/short-benchmark/20241208T172603_9207eaf/blocks-import-benchmark.csv
This option can be used like this
I've also refactored to add a
main()
.It would be easier to review this PR in Github by hiding whitespace like this https://github.com/status-im/nimbus-eth1/pull/2982/files?w=1