feat: allow comments in config.json, genesis.json, node_key.json and validator_key.json #8423
Conversation
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.
| @@ -271,7 +272,9 @@ impl GenesisConfig { | |||
| pub fn from_file<P: AsRef<Path>>(path: P) -> anyhow::Result<Self> { | |||
| 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`. |
nearcore/src/dyn_config.rs
Outdated
| 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`). |
ppca
changed the title
ppca
changed the title
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.
👍
ppca
force-pushed
the
xiangyiz/ND-271
branch
2 times, most recently
from
8ef8e2d to
693e452
Compare
…lidator_key.json and node_key.json
…e function from near-config-utils
…ons for genesis config
… toml in workspace
ppca
changed the title
ppca
changed the title
ppca
changed the title
ppca
changed the title
…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.