-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: store logs in different folders based on the chain #3948
Conversation
Codecov Report
... and 8 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
looks like a good start! is it possible to get a CLI test for this?
for example like the ones here:
Lines 879 to 890 in f3a7ae1
#[test] | |
fn parse_db_path() { | |
let cmd = Command::try_parse_from(["reth"]).unwrap(); | |
let data_dir = cmd.datadir.unwrap_or_chain_default(cmd.chain.chain); | |
let db_path = data_dir.db_path(); | |
assert!(db_path.ends_with("reth/mainnet/db"), "{:?}", cmd.config); | |
let cmd = Command::try_parse_from(["reth", "--datadir", "my/custom/path"]).unwrap(); | |
let data_dir = cmd.datadir.unwrap_or_chain_default(cmd.chain.chain); | |
let db_path = data_dir.db_path(); | |
assert_eq!(db_path, Path::new("my/custom/path/db")); | |
} |
At the end I chose the following option:
This actually turned out the best decision because you can delete some fields to the I also added a test to verify that the directory is properly parsed. Let me know what do you think. |
1182d39
to
27e84c0
Compare
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.
Makes sense to me, thank you. There are some conflicts to solve
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.
Makes sense and looks good to me! thank you!
Closes #3858
When
--log.persistent
flag is enabled, logs are now stored in different folders based on the chain or in the already-chain-specific--datadir
.Examples (no --datadir flag):
~/.cache/reth/logs/mainnet/reth.log
~/.cache/reth/logs/sepolia/reth.log
Examples (with --datadir flag):
~/custom-data-dir/reth.log