scripts: add consensus log downloader config infrastructure and args parsing#12839
scripts: add consensus log downloader config infrastructure and args parsing#12839lev-starkware merged 1 commit intomainfrom
Conversation
405ed20 to
2c432a1
Compare
9ee0e5f to
c3e9bf7
Compare
c3e9bf7 to
90cbc68
Compare
lev-starkware
left a comment
There was a problem hiding this comment.
@lev-starkware made 3 comments and resolved 4 discussions.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on TzahiTaub).
e06a1b4 to
99e0f64
Compare
90cbc68 to
456fd32
Compare
TzahiTaub
left a comment
There was a problem hiding this comment.
@TzahiTaub reviewed 2 files and all commit messages, and made 11 comments.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on lev-starkware).
scripts/consensus_report/.gitignore line 1 at r4 (raw file):
env_map.yaml
Suggestion:
env_map.yaml # Should be supplied by the user and never committed.scripts/consensus_report/.gitignore line 7 at r4 (raw file):
*.pyc .venv/ venv/
This is needed if you plan to create a venv here as a part of the script. If you do, pick one of the names, no need for both. If you don't, delete both.
Code quote:
.venv/
venv/scripts/consensus_report/generate_logs_and_report.py line 16 at r4 (raw file):
class EnvConfig: project: str namespace_re: str
Suggestion:
namespace_regexscripts/consensus_report/generate_logs_and_report.py line 36 at r4 (raw file):
environments = data.get("environments", {}) default_env = data.get("default_environment", "integration")
I think it's better to remove this and let the user always state the env explicitly.
scripts/consensus_report/generate_logs_and_report.py line 57 at r4 (raw file):
ap.add_argument("--height", required=True, type=int, help="Block height, e.g. 6591090") ap.add_argument( "--out_json",
Suggestion:
"--out_json_path",scripts/consensus_report/generate_logs_and_report.py line 67 at r4 (raw file):
ap.add_argument("--start", help="RFC3339 timestamp - time window start (requires --end)") ap.add_argument("--end", help="RFC3339 timestamp - time window end (requires --start)") ap.add_argument("--near", help="RFC3339 timestamp - search near this time (±2h window)")
Give the format and an example here instead of the RFC to avoid an additional check of what RFC3339 is.
- I prefer to have the simplest to use, good enough format, this one seems too detailed for our needs. Wrote in the diff what I would expect. Same for the other two.
Suggestion:
ap.add_argument("--near", help="YY-MM-DD HH::MM formated timestamp- search near this time (±2h window)")scripts/consensus_report/generate_logs_and_report.py line 71 at r4 (raw file):
"--today", action="store_true", help="Use today's UTC midnight-to-midnight time window (default if no time args)",
What does UTC here means? I we run this script at local time (utc+2) of 01:59 == 23:59 utc will we get the last two hours data (this is what we want) or the data of yesterday?
Code quote:
help="Use today's UTC midnight-to-midnight time window (default if no time args)",scripts/consensus_report/generate_logs_and_report.py line 72 at r4 (raw file):
action="store_true", help="Use today's UTC midnight-to-midnight time window (default if no time args)", )
I think last 2 hours (or some other time) is better - this will probably be the used option when debugging in real time, and if we look at a consensus height that is around midnight, the "today" will give partial data.
As a note - let the script error if the logs we retrieve miss the start log of the height, and print a bold errorif we miss the next height start log - so that a user will now it get partial data. (I think it shouldn't error in the latter as we might want to run this during a height that is stuck).
Code quote:
ap.add_argument(
"--today",
action="store_true",
help="Use today's UTC midnight-to-midnight time window (default if no time args)",
)scripts/consensus_report/generate_logs_and_report.py line 79 at r4 (raw file):
) ap.add_argument( "--report",
Suggestion:
"--out_report_path",scripts/consensus_report/generate_logs_and_report.py line 80 at r4 (raw file):
ap.add_argument( "--report", help="Generate report to this file (requires --out_json). Extension .txt added if missing.",
I think it's more convenient to have the option to avoid the--out_json ' flag if we call --report. Have the out_json` the same as the report name (with the diff in extension) if not provided explicitly.
Suggestion:
help="Report file path to generate. Extension .txt is added if missing.",scripts/consensus_report/generate_logs_and_report.py line 71 at r4 (raw file):
"--today", action="store_true", help="Use today's UTC midnight-to-midnight time window (default if no time args)",
Add assertions for the input - e.g, start and end come together, one of this group or auto must provided.
Code quote:
ap.add_argument("--start", help="RFC3339 timestamp - time window start (requires --end)")
ap.add_argument("--end", help="RFC3339 timestamp - time window end (requires --start)")
ap.add_argument("--near", help="RFC3339 timestamp - search near this time (±2h window)")
ap.add_argument(
"--today",
action="store_true",
help="Use today's UTC midnight-to-midnight time window (default if no time args)",
lev-starkware
left a comment
There was a problem hiding this comment.
@lev-starkware made 10 comments and resolved 1 discussion.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on TzahiTaub).
scripts/consensus_report/.gitignore line 7 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
This is needed if you plan to create a venv here as a part of the script. If you do, pick one of the names, no need for both. If you don't, delete both.
Done.
scripts/consensus_report/generate_logs_and_report.py line 36 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
I think it's better to remove this and let the user always state the env explicitly.
Done.
scripts/consensus_report/generate_logs_and_report.py line 67 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
Give the format and an example here instead of the RFC to avoid an additional check of what
RFC3339is.
- I prefer to have the simplest to use, good enough format, this one seems too detailed for our needs. Wrote in the diff what I would expect. Same for the other two.
Done. As discussed
scripts/consensus_report/generate_logs_and_report.py line 71 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
Add assertions for the input - e.g, start and end come together, one of this group or
automust provided.
Done.
scripts/consensus_report/generate_logs_and_report.py line 71 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
What does
UTChere means? I we run this script at local time (utc+2) of 01:59 == 23:59 utc will we get the last two hours data (this is what we want) or the data of yesterday?
Done.
scripts/consensus_report/generate_logs_and_report.py line 72 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
I think
last 2 hours(or some other time) is better - this will probably be the used option when debugging in real time, and if we look at a consensus height that is around midnight, the "today" will give partial data.
As a note - let the script error if the logs we retrieve miss the start log of the height, and print a bold errorif we miss the next height start log - so that a user will now it get partial data. (I think it shouldn't error in the latter as we might want to run this during a height that is stuck).
Done. changed to last-24-hours
scripts/consensus_report/generate_logs_and_report.py line 80 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
I think it's more convenient to have the option to avoid the
--out_json ' flag if we call--report. Have theout_json` the same as the report name (with the diff in extension) if not provided explicitly.
Done. Already done, only forgot to change a message.
scripts/consensus_report/.gitignore line 1 at r4 (raw file):
env_map.yaml
Done.
scripts/consensus_report/generate_logs_and_report.py line 57 at r4 (raw file):
ap.add_argument("--height", required=True, type=int, help="Block height, e.g. 6591090") ap.add_argument( "--out_json",
Done.
scripts/consensus_report/generate_logs_and_report.py line 79 at r4 (raw file):
) ap.add_argument( "--report",
Done. Agreed on --report_path
456fd32 to
91f61b8
Compare
99e0f64 to
69b4c8d
Compare
TzahiTaub
left a comment
There was a problem hiding this comment.
@TzahiTaub reviewed 1 file and all commit messages, made 4 comments, and resolved 8 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on lev-starkware).
scripts/consensus_report/.gitignore line 1 at r4 (raw file):
Previously, lev-starkware wrote…
Done.
Not pushed
scripts/consensus_report/.gitignore line 7 at r4 (raw file):
Previously, lev-starkware wrote…
Done.
Not pushed
scripts/consensus_report/generate_logs_and_report.py line 71 at r5 (raw file):
ap.add_argument( "--start", help="TIMESTAMP - time window start (requires --end). TIMESTAMP format: YYYY-MM-DDTHH:MM:SSZ",
The Z stands for UTC right? We want to use local time. Please add to the help message as well - "in local time".
Code quote:
TIMESTAMP format: YYYY-MM-DDTHH:MM:SSZ",91f61b8 to
cd936a1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
cd936a1 to
b718a83
Compare
lev-starkware
left a comment
There was a problem hiding this comment.
@lev-starkware made 4 comments and resolved 3 discussions.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on TzahiTaub).
scripts/consensus_report/.gitignore line 1 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
Not pushed
Done.
scripts/consensus_report/.gitignore line 7 at r4 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
Not pushed
Done.
scripts/consensus_report/generate_logs_and_report.py line 71 at r4 (raw file):
Previously, lev-starkware wrote…
Done.
In following PRs.
scripts/consensus_report/generate_logs_and_report.py line 71 at r5 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
The
Zstands for UTC right? We want to use local time. Please add to the help message as well - "in local time".
Done.
TzahiTaub
left a comment
There was a problem hiding this comment.
@TzahiTaub reviewed 2 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on lev-starkware).

Note
Low Risk
Adds a new standalone script and ignore rules without wiring it into production code paths. Main risk is minor: argument/config validation behavior and dependency on
env_map.yaml/PyYAML for local tooling.Overview
Introduces a new
scripts/consensus_reportscaffold for generating consensus log reports.Adds
generate_logs_and_report.pywithenv_map.yamlloading/validation (viayaml.safe_load) and a CLI that accepts environment selection plus block height and time-window/report output options; core log download/report generation is left as TODOs. Also adds a local.gitignoreto prevent committingenv_map.yamland generated*.json/*.txtartifacts.Written by Cursor Bugbot for commit b718a83. This will update automatically on new commits. Configure here.