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

Some improve #11

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Some improve #11

wants to merge 6 commits into from

Conversation

lyfsn
Copy link

@lyfsn lyfsn commented Jun 7, 2024

Hey, there's a small change when I use it, and I think it might be a problem. Please review it and check if it's correct.

For fix #3: On some machines, if the hz_actual_friendly field is not present in the CPU info, it will cause the whole test script to fail. So, getting the CPU frequency using psutil might be better.

I tried to add a new startup parameter --o so that you can specify the result folder location. This is useful because if you run the test multiple times on the same machine, using this flag can help store multiple results in the same project.

About the report folder, I see the script creates results/reports in the results directory but does not use it, and currently generates reports in the project's root folder. Considering the addition of the --o flag, I think all reports should follow the results directory. So, modify the reports location to be within results.

The previous JSON format in the --imageBulk argument had a syntax error due to an extra comma and quote. This caused a JSONDecodeError during script execution. The JSON string is now correctly formatted, ensuring successful parsing.

root@Ubuntu-2204-jammy-amd64-base ~/gas-benchmarks # python3 setup_node.py --client nethermind
image Bulk: {"nethermind": "default", "besu": "default", "geth": "default", "reth": "default"}, "erigon": "default"}
Traceback (most recent call last):
  File "/root/gas-benchmarks/setup_node.py", line 86, in <module>
    main()
  File "/root/gas-benchmarks/setup_node.py", line 67, in main
    images_json = json.loads(images_bulk)
  File "/usr/lib/python3.10/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.10/json/decoder.py", line 340, in decode
    raise JSONDecodeError("Extra data", s, end)
json.decoder.JSONDecodeError: Extra data: line 1 column 83 (char 82)

Since the response file is appended at the end, I think we should base the comment on "get the latest line" by reading the last line of the file. The first line of the file might have a "SYNCING" status.

I'm not sure if these changes are satisfactory for this project, as I made the process easier for my use. Please review which changes are good for the project, and I will remove the incorrect changes.

This PR code has already run and generated results there: https://github.com/lyfsn/gas-benchmarks/actions/runs/9411197574

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeyError: 'hz_actual_friendly'
1 participant