feat: allow comments in config.json, genesis.json, node_key.json and validator_key.json - #8423
Conversation
nikurt
left a comment
There was a problem hiding this comment.
Should we also allow comments in node_key.json, i.e. https://github.com/near/nearcore/blob/master/nearcore/src/config.rs#L1293 ?
Should we also allow comments in validator_key.json, i.e. https://github.com/near/nearcore/blob/master/core/primitives/src/validator_signer.rs#L163 ?
nearup
I don't think that updating nearup is needed. nearup is not the recommended way of running nodes. If comments are not allowed in those conditions, that is acceptable.
| let file = File::open(path).with_context(|| "Could not open genesis config file.")?; | ||
| let reader = BufReader::new(file); | ||
| let genesis_config: GenesisConfig = serde_json::from_reader(reader) | ||
| // Strip the comments from the input (use `as_bytes()` to get a `Read`). |
There was a problem hiding this comment.
| // Strip the comments from the input (use `as_bytes()` to get a `Read`). | |
| // Strip the comments from the input. | |
| // `as_bytes()` returns a `Read`. |
| tracing::info!(target: "neard", config=?config, "Changing the config {path:?}."); | ||
| return Ok(Some(config)); | ||
| Ok(config_str) => { | ||
| // Strip the comments from the input (use `as_bytes()` to get a `Read`). |
I've tested for dynamic config field The strip comments for the other json's should also work but not sure how we can test them? |
Add comments to |
| @@ -261,18 +261,25 @@ impl GenesisConfig { | |||
| /// | |||
There was a problem hiding this comment.
Add a comment that it's not JSON string, but JSON-with-comments string.
| @@ -395,11 +395,12 @@ impl Default for Config { | |||
|
|
|||
| impl Config { | |||
| pub fn from_file(path: &Path) -> anyhow::Result<Self> { | |||
There was a problem hiding this comment.
Comment that the file can be JSON-with-comments.
| @@ -0,0 +1,11 @@ | |||
| [package] | |||
There was a problem hiding this comment.
Add this in the global Cargo.toml as a workspace member. Around line 60.
|
|
||
| use json_comments::StripComments; | ||
|
|
||
| pub fn strip_comments_from_json_str(json_str: &String) -> std::io::Result<String> { |
There was a problem hiding this comment.
Good idea to move all the code that deals with json_comments to a separate module.
👍
8ef8e2d to
693e452
Compare
…lidator_key.json and node_key.json
…e function from near-config-utils
…ons for genesis config
… toml in workspace
…validator.json (#8423) * Use json-comments-rs crate to skip config file comments * strip comments out from json for genesis_config.json, config.json, validator_key.json and node_key.json * strip comments functions sit in near-config-utils (utils/config)
|
What motivation for having comments in JSON? Can I remove this feature? With this feature, we have higher startup time. |
|
The PR description could have been more explicit about it. It greatly speeds up development of features involving config changes. |
This is useful for dynamic config.
For this to work for node start, nearup also needs to be updated.