Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
made suggested changes, and removed some deprecated flags and options
Browse files Browse the repository at this point in the history
  • Loading branch information
wesrer committed Jun 1, 2020
1 parent 35dca38 commit 1315141
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 143 deletions.
106 changes: 39 additions & 67 deletions parity/cli/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ pub struct Args {
pub arg_jsonrpc_interface: String,
pub arg_jsonrpc_apis: String,
pub arg_jsonrpc_hosts: String,
pub arg_jsonrpc_threads: Option<usize>,
pub arg_jsonrpc_server_threads: Option<usize>,
pub arg_jsonrpc_cors: String,
pub arg_jsonrpc_max_payload: Option<usize>,
Expand Down Expand Up @@ -282,7 +281,7 @@ impl Args {
let (default_config, fallback_config) =
Args::generate_default_configuration(default_config_path, fallback_config_path)?;

args.from_cli(raw_input, default_config, fallback_config)?;
args.absorb_cli(raw_input, default_config, fallback_config)?;

Ok(args)
}
Expand All @@ -294,14 +293,17 @@ impl Args {
globals.convenience.config_generate = None;

if let Some(path) = &config_generate {
let current_flags = match toml::to_string(globals) {
Ok(x) => x,
Err(_) => return Err(ArgsError::ConfigWriteError("Failed to generate valid config toml from current flags. Please report a bug if this error persists.".to_owned())),
};

if let Err(_) = fs::write(&path, current_flags) {
return Err(ArgsError::ConfigParseError(format!("Failed to write config to given file path: {}. Please try again with a valid path and config name.", &path)));
};
let current_flags = toml::to_string(globals).map_err(|e| {
ArgsError::ConfigWriteError(format!(
"Failed to generate valid config toml from current flags: {}.Please report a bug if this error persists.", e
))
})?;

fs::write(&path, current_flags).map_err(|_| {
ArgsError::ConfigParseError(format!(
"Failed to write config to given file path {}. Please try again with a valid path and config name.", &path
))
})?;
}
Ok(())
}
Expand All @@ -314,40 +316,35 @@ impl Args {
let default_config_file:String = get_config(default_config_path)?;
let fallback_config_file:String = get_config(fallback_config_path)?;

let default_config: Globals = match toml::from_str(&default_config_file) {
Ok(x) => x,
Err(e) => {
return Err(ArgsError::ConfigParseError(format!(
"Failure to parse config file: {}, error: {}",
default_config_path, e
)))
}
};
let fallback_config: Globals = match toml::from_str(&fallback_config_file) {
Ok(x) => x,
Err(_) => {
return Err(ArgsError::ConfigParseError(format!(
"Failure to parse config file: {}",
fallback_config_path
)))
}
};
let default_config: Globals = toml::from_str(&default_config_file).map_err(|e| {
ArgsError::ConfigParseError(format!(
"Failure to parse config file: {}, error: {}",
default_config_path, e
))
})?;

let fallback_config: Globals = toml::from_str(&fallback_config_file).map_err(|e| {
ArgsError::ConfigParseError(format!(
"Failure to parse config file {}: {}",
fallback_config_path, e
))
})?;

Ok((default_config, fallback_config))
}

pub fn from_cli(
pub fn absorb_cli(
&mut self,
cli_args: ArgsInput,
default_globals: Globals,
fallback_globals: Globals,
) -> Result<(), ArgsError> {
self.from_subcommands(&cli_args)?;
self.from_globals(cli_args, default_globals, fallback_globals)?;
self.absorb_subcommands(&cli_args)?;
self.absorb_globals(cli_args, default_globals, fallback_globals)?;
Ok(())
}

fn from_subcommands(&mut self, cli_args: &ArgsInput) -> Result<(), ArgsError> {
fn absorb_subcommands(&mut self, cli_args: &ArgsInput) -> Result<(), ArgsError> {
match &cli_args.subcommands {
None => {}
Some(subcommand) => match &subcommand {
Expand Down Expand Up @@ -425,13 +422,6 @@ impl Args {
}
}
}
SubCommands::Tools(t) => {
self.cmd_tools = true;
self.cmd_tools_hash = true;

let Tools::Hash { file } = t;
self.arg_tools_hash_file = (*file).clone();
}
SubCommands::Restore(r) => {
self.cmd_restore = true;
self.arg_restore_file = r.file.clone();
Expand All @@ -456,44 +446,31 @@ impl Args {
SubCommands::ExportHardcodedSync => {
self.cmd_export_hardcoded_sync = true;
}
SubCommands::Dapp(dapp) => {
self.cmd_dapp = true;
self.arg_dapp_path = dapp.path.clone();
}

},
}
Ok(())
}

fn select_value<T>(raw: Option<T>, default: Option<T>, fallback: Option<T>) -> T {
match (raw, default, fallback) {
(Some(x), _, _) => x,
(None, Some(x), _) => x,
(None, None, Some(x)) => x,
_ => todo!(), // FIXME: this case should never arise, but we need to fail gracefully if it does
}
raw.or(default).or(fallback).expect("Value is always present, at least in fallback")
}

fn select_option<T>(raw: Option<T>, default: Option<T>, fallback: Option<T>) -> Option<T> {
match (&raw, &default, &fallback) {
(None, None, Some(_)) => fallback,
(None, Some(_), _) => default,
// Handles the cases where there was a raw value, so we can ignore everything else
_ => raw,
}
raw.or(default).or(fallback)
}

fn from_globals(
fn absorb_globals(
&mut self,
cli_args: ArgsInput,
defaults: Globals,
fallback: Globals,
) -> Result<(), ArgsError> {
self.flags_from_globals(&cli_args, &defaults, &fallback);
self.options_from_globals(cli_args, defaults, fallback);
self.absorb_global_flags(&cli_args, &defaults, &fallback);
self.absorb_global_options(cli_args, defaults, fallback);

if let (Some(min_peers), Some(max_peers)) = (self.arg_min_peers, self.arg_max_peers) {
if (max_peers >= min_peers).not() {
if (max_peers < min_peers){
return Err(ArgsError::PeerConfigurationError(
"max-peers need to be greater than or equal to min-peers".to_owned(),
));
Expand All @@ -502,12 +479,12 @@ impl Args {
Ok(())
}

fn options_from_globals(&mut self, cli_args: ArgsInput, defaults: Globals, fallback: Globals) {
fn absorb_global_options(&mut self, cli_args: ArgsInput, defaults: Globals, fallback: Globals) {
// Unnatural cases

self.arg_ipc_path = Args::select_value(
cli_args.globals.ipc.ipc_path,
IPCOptions::ipc_path_default(),
Some(IPCOptions::ipc_path_default()),
None, // We don't care about fallback in this case, since the previous operation is infallible
);
self.arg_password = cli_args.globals.account.password;
Expand Down Expand Up @@ -715,11 +692,6 @@ impl Args {
defaults.http_json_rpc.jsonrpc_hosts,
fallback.http_json_rpc.jsonrpc_hosts,
);
self.arg_jsonrpc_threads = Args::select_option(
cli_args.globals.http_json_rpc.jsonrpc_threads,
defaults.http_json_rpc.jsonrpc_threads,
fallback.http_json_rpc.jsonrpc_threads,
);
self.arg_jsonrpc_server_threads = Args::select_option(
cli_args.globals.http_json_rpc.jsonrpc_server_threads,
defaults.http_json_rpc.jsonrpc_server_threads,
Expand Down Expand Up @@ -1114,7 +1086,7 @@ impl Args {
);
}

fn flags_from_globals(&mut self, cli_args: &ArgsInput, defaults: &Globals, fallback: &Globals) {
fn absorb_global_flags(&mut self, cli_args: &ArgsInput, defaults: &Globals, fallback: &Globals) {
self.arg_enable_signing_queue = cli_args.globals.account.enable_signing_queue
|| defaults.account.enable_signing_queue
|| fallback.account.enable_signing_queue;
Expand Down
15 changes: 8 additions & 7 deletions parity/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ use rust_embed::RustEmbed;
struct Config;

pub fn get_config(config_name: &str) -> Result<String, ArgsError> {
match Config::get(config_name) {
Some(x) => Ok((std::str::from_utf8(x.as_ref()).unwrap()).to_owned()),
None => Err(ArgsError::ConfigReadError(format!(
"Failure to read config file: {}",
config_name
))),
}
Config::get(config_name)
.ok_or_else(|| "does not exist".to_owned())
// .and_then(|x| srt(x).map_err(|e| e.to_owned()))
.and_then(|x| String::from_utf8(x.to_vec()).map_err(|e| e.to_string()))
.map_err(|e| ArgsError::ConfigReadError(format!(
"Failure to read config file {}: {}",
config_name, e
)))
}
17 changes: 5 additions & 12 deletions parity/cli/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ pub struct PrivateTransactions {

#[structopt(
long = "private-signer",
long = "ACCOUNT",
name = "ACCOUNT",
help = "Specify the account for signing public transaction created upon verified private transaction."
)]
pub private_signer: Option<String>,
Expand Down Expand Up @@ -457,11 +457,11 @@ pub struct IPCOptions {
}

impl IPCOptions {
pub fn ipc_path_default() -> Option<String> {
pub fn ipc_path_default() -> String {
if cfg!(windows) {
Some(r"\\.\pipe\jsonrpc.ipc".to_string())
r"\\.\pipe\jsonrpc.ipc".to_owned()
} else {
Some("$BASE/jsonrpc.ipc".to_string())
"$BASE/jsonrpc.ipc".to_owned()
}
}
}
Expand Down Expand Up @@ -517,13 +517,6 @@ pub struct HTTP_JSON_RPC_Options {
)]
pub jsonrpc_hosts: Option<String>,

#[structopt(
long = "jsonrpc-threads",
name = "JSONRPC_THREADS_NUM",
help = "DEPRECATED, DOES NOTHING"
)]
pub jsonrpc_threads: Option<usize>,

#[structopt(
long = "jsonrpc-server-threads",
name = "JSONRPC_SERVER_THREADS",
Expand Down Expand Up @@ -633,7 +626,7 @@ pub struct WebsocketsOptions {

#[structopt(
help = "Maximum number of allowed concurrent WebSockets JSON-RPC connections.",
long = "ws=-connections",
long = "ws-connections",
name = "WS_MAX_CONN"
)]
pub ws_max_connections: Option<usize>,
Expand Down
19 changes: 0 additions & 19 deletions parity/cli/subcommands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ pub enum SubCommands {
Signer(Signer),
Snapshots(Snapshots),
Restore(Restore),
Tools(Tools),
Db(Db),
#[structopt(
about = "Print the hashed light clients headers of the given --chain (default: mainnet) in a JSON format. To be used as hardcoded headers in a genesis file."
)]
ExportHardcodedSync,
Dapp(Dapp),
}

#[derive(StructOpt, Deserialize, Debug, Clone)]
Expand Down Expand Up @@ -208,16 +206,6 @@ pub struct Restore {
pub file: Option<String>,
}

#[derive(StructOpt, Deserialize, Debug, Clone)]
#[structopt(about = "Tools")]
pub enum Tools {
#[structopt(about = "Hash a file using the Keccak-256 algorithm")]
Hash {
#[structopt(name = "FILE")]
file: Option<String>,
},
}

#[derive(StructOpt, Deserialize, Debug, Clone)]
#[structopt(about = "Manage the Database representing the state of the blockchain on this system")]
pub enum Db {
Expand All @@ -233,10 +221,3 @@ pub enum Db {
num: u32,
},
}

#[derive(StructOpt, Deserialize, Debug, Clone)]
#[structopt(about = "Manage Dapps")]
pub struct Dapp {
#[structopt(help = "Path to the dapps", name = "PATH")]
pub path: Option<String>,
}
4 changes: 2 additions & 2 deletions parity/cli/tests/test_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn test_overwrite_custom_config_with_raw_flags() {
let (user_defaults, fallback) =
Args::generate_default_configuration("test_config.toml", "config_default.toml").unwrap();

resolved.from_cli(raw, user_defaults, fallback);
resolved.absorb_cli(raw, user_defaults, fallback);

assert_eq!(resolved.arg_stratum_secret, Some("Changed".to_owned()));
}
Expand All @@ -50,7 +50,7 @@ fn test_not_accepting_min_peers_bigger_than_max_peers() {
raw.globals.networking.min_peers = Some(50);
raw.globals.networking.max_peers = Some(40);

let output = resolved.from_cli(raw, user_defaults, fallback);
let output = resolved.absorb_cli(raw, user_defaults, fallback);

assert_eq!(
output,
Expand Down
Loading

0 comments on commit 1315141

Please sign in to comment.