From 0541e4fe33c613ee5bd4233d2a3d404930dc1004 Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 14 Oct 2017 10:59:52 +0200 Subject: [PATCH 1/4] Refactored config parsing and config writing into separate methods --- programs/witness_node/main.cpp | 137 ++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 64 deletions(-) diff --git a/programs/witness_node/main.cpp b/programs/witness_node/main.cpp index b2b1bc89d8..7d17de0d33 100644 --- a/programs/witness_node/main.cpp +++ b/programs/witness_node/main.cpp @@ -61,6 +61,77 @@ namespace bpo = boost::program_options; void write_default_logging_config_to_stream(std::ostream& out); fc::optional load_logging_config_from_ini_file(const fc::path& config_ini_filename); +static void load_config_file( const fc::path& config_ini_path, const bpo::options_description& cfg_options, + bpo::variables_map& options ) +{ + boost::container::flat_set seen; + bpo::options_description unique_options("Graphene Witness Node"); + for( const boost::shared_ptr od : cfg_options.options() ) + { + const std::string name = od->long_name(); + if( seen.find(name) != seen.end() ) continue; + seen.insert(name); + unique_options.add( od ); + } + + // get the basic options + bpo::store(bpo::parse_config_file(config_ini_path.preferred_string().c_str(), + unique_options, true), options); + + // try to get logging options from the config file. + try + { + fc::optional logging_config = load_logging_config_from_ini_file(config_ini_path); + if (logging_config) + fc::configure_logging(*logging_config); + } + catch (const fc::exception&) + { + wlog("Error parsing logging config from config file ${config}, using default config", ("config", config_ini_path.preferred_string())); + } +} + +static void create_new_config_file( const fc::path& config_ini_path, const fc::path& data_dir, + const bpo::options_description& cfg_options ) +{ + ilog("Writing new config file at ${path}", ("path", config_ini_path)); + if( !fc::exists(data_dir) ) + fc::create_directories(data_dir); + + boost::container::flat_set seen; + std::ofstream out_cfg(config_ini_path.preferred_string()); + for( const boost::shared_ptr od : cfg_options.options() ) + { + if( !od->description().empty() ) + out_cfg << "# " << od->description() << "\n"; + boost::any store; + if( !od->semantic()->apply_default(store) ) + out_cfg << "# " << od->long_name() << " = \n"; + else { + const std::string name = od->long_name(); + if( seen.find(name) != seen.end() ) continue; + seen.insert(name); + auto example = od->format_parameter(); + if( example.empty() ) + // This is a boolean switch + out_cfg << od->long_name() << " = " << "false\n"; + else { + // The string is formatted "arg (=)" + example.erase(0, 6); + example.erase(example.length()-1); + out_cfg << od->long_name() << " = " << example << "\n"; + } + } + out_cfg << "\n"; + } + write_default_logging_config_to_stream(out_cfg); + out_cfg.close(); + // read the default logging config we just wrote out to the file and start using it + fc::optional logging_config = load_logging_config_from_ini_file(config_ini_path); + if (logging_config) + fc::configure_logging(*logging_config); +} + int main(int argc, char** argv) { app::application* node = new app::application(); fc::oexception unhandled_exception; @@ -110,71 +181,9 @@ int main(int argc, char** argv) { fc::path config_ini_path = data_dir / "config.ini"; if( fc::exists(config_ini_path) ) - { - boost::container::flat_set seen; - bpo::options_description unique_options("Graphene Witness Node"); - for( const boost::shared_ptr od : cfg_options.options() ) - { - const std::string name = od->long_name(); - if( seen.find(name) != seen.end() ) continue; - seen.insert(name); - unique_options.add( od ); - } - - // get the basic options - bpo::store(bpo::parse_config_file(config_ini_path.preferred_string().c_str(), unique_options, true), options); - - // try to get logging options from the config file. - try - { - fc::optional logging_config = load_logging_config_from_ini_file(config_ini_path); - if (logging_config) - fc::configure_logging(*logging_config); - } - catch (const fc::exception&) - { - wlog("Error parsing logging config from config file ${config}, using default config", ("config", config_ini_path.preferred_string())); - } - } + load_config_file( config_ini_path, cfg_options, options ); else - { - ilog("Writing new config file at ${path}", ("path", config_ini_path)); - if( !fc::exists(data_dir) ) - fc::create_directories(data_dir); - - boost::container::flat_set seen; - std::ofstream out_cfg(config_ini_path.preferred_string()); - for( const boost::shared_ptr od : cfg_options.options() ) - { - if( !od->description().empty() ) - out_cfg << "# " << od->description() << "\n"; - boost::any store; - if( !od->semantic()->apply_default(store) ) - out_cfg << "# " << od->long_name() << " = \n"; - else { - const std::string name = od->long_name(); - if( seen.find(name) != seen.end() ) continue; - seen.insert(name); - auto example = od->format_parameter(); - if( example.empty() ) - // This is a boolean switch - out_cfg << od->long_name() << " = " << "false\n"; - else { - // The string is formatted "arg (=)" - example.erase(0, 6); - example.erase(example.length()-1); - out_cfg << od->long_name() << " = " << example << "\n"; - } - } - out_cfg << "\n"; - } - write_default_logging_config_to_stream(out_cfg); - out_cfg.close(); - // read the default logging config we just wrote out to the file and start using it - fc::optional logging_config = load_logging_config_from_ini_file(config_ini_path); - if (logging_config) - fc::configure_logging(*logging_config); - } + create_new_config_file( config_ini_path, data_dir, cfg_options ); bpo::notify(options); node->initialize(data_dir, options); From b50fa81d389901b7ad025e8a2a264f2c7147ae24 Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 14 Oct 2017 11:04:48 +0200 Subject: [PATCH 2/4] Swap logic: if config file does not exist, create one and parse it Previously, a newly written config file was ignored. For the included plugins that didn't matter though, because all config file options (and their default values) are also CLI options, and CLI defaults always apply, no matter if a config file exists or not. Now, a new config file is written with the default option values (if any), and parsed immediately after that. In other words, the config defaults are applied again. This does not result in a different outcome, at least with the included plugins. --- programs/witness_node/main.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/programs/witness_node/main.cpp b/programs/witness_node/main.cpp index 7d17de0d33..0fc8e22fdf 100644 --- a/programs/witness_node/main.cpp +++ b/programs/witness_node/main.cpp @@ -180,10 +180,9 @@ int main(int argc, char** argv) { } fc::path config_ini_path = data_dir / "config.ini"; - if( fc::exists(config_ini_path) ) - load_config_file( config_ini_path, cfg_options, options ); - else + if( !fc::exists(config_ini_path) ) create_new_config_file( config_ini_path, data_dir, cfg_options ); + load_config_file( config_ini_path, cfg_options, options ); bpo::notify(options); node->initialize(data_dir, options); From 49a9cc69f3e8563d50d4a34ac1406fa907bf8ea8 Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 14 Oct 2017 18:41:13 +0200 Subject: [PATCH 3/4] Refactored option deduplication into a separate class --- programs/witness_node/main.cpp | 39 +++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/programs/witness_node/main.cpp b/programs/witness_node/main.cpp index 0fc8e22fdf..18d1f8d82f 100644 --- a/programs/witness_node/main.cpp +++ b/programs/witness_node/main.cpp @@ -61,18 +61,33 @@ namespace bpo = boost::program_options; void write_default_logging_config_to_stream(std::ostream& out); fc::optional load_logging_config_from_ini_file(const fc::path& config_ini_filename); +class deduplicator +{ + public: + boost::shared_ptr next(const boost::shared_ptr& o) + { + const std::string name = o->long_name(); + if( seen.find( name ) != seen.end() ) + return nullptr; + seen.insert(name); + return o; + } + + private: + boost::container::flat_set seen; +}; + static void load_config_file( const fc::path& config_ini_path, const bpo::options_description& cfg_options, bpo::variables_map& options ) { - boost::container::flat_set seen; + deduplicator dedup; bpo::options_description unique_options("Graphene Witness Node"); - for( const boost::shared_ptr od : cfg_options.options() ) + for( const boost::shared_ptr opt : cfg_options.options() ) { - const std::string name = od->long_name(); - if( seen.find(name) != seen.end() ) continue; - seen.insert(name); - unique_options.add( od ); - } + boost::shared_ptr od = dedup.next(opt); + if( !od ) continue; + unique_options.add( od ); + } // get the basic options bpo::store(bpo::parse_config_file(config_ini_path.preferred_string().c_str(), @@ -98,19 +113,19 @@ static void create_new_config_file( const fc::path& config_ini_path, const fc::p if( !fc::exists(data_dir) ) fc::create_directories(data_dir); - boost::container::flat_set seen; + deduplicator dedup; std::ofstream out_cfg(config_ini_path.preferred_string()); - for( const boost::shared_ptr od : cfg_options.options() ) + for( const boost::shared_ptr opt : cfg_options.options() ) { + boost::shared_ptr od = dedup.next(opt); + if( !od ) continue; + if( !od->description().empty() ) out_cfg << "# " << od->description() << "\n"; boost::any store; if( !od->semantic()->apply_default(store) ) out_cfg << "# " << od->long_name() << " = \n"; else { - const std::string name = od->long_name(); - if( seen.find(name) != seen.end() ) continue; - seen.insert(name); auto example = od->format_parameter(); if( example.empty() ) // This is a boolean switch From 95097a574940c7d9292221c1c6c85cf7810856ad Mon Sep 17 00:00:00 2001 From: Peter Conrad Date: Sat, 14 Oct 2017 21:25:35 +0200 Subject: [PATCH 4/4] Use different defaults for some options when writing the config file --- programs/witness_node/main.cpp | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/programs/witness_node/main.cpp b/programs/witness_node/main.cpp index 18d1f8d82f..20ef8d7bb7 100644 --- a/programs/witness_node/main.cpp +++ b/programs/witness_node/main.cpp @@ -64,17 +64,23 @@ fc::optional load_logging_config_from_ini_file(const fc::pat class deduplicator { public: - boost::shared_ptr next(const boost::shared_ptr& o) + deduplicator() : modifier(nullptr) {} + + deduplicator(const boost::shared_ptr (*mod_fn)(const boost::shared_ptr&)) + : modifier(mod_fn) {} + + const boost::shared_ptr next(const boost::shared_ptr& o) { const std::string name = o->long_name(); if( seen.find( name ) != seen.end() ) return nullptr; seen.insert(name); - return o; + return modifier ? modifier(o) : o; } private: boost::container::flat_set seen; + const boost::shared_ptr (*modifier)(const boost::shared_ptr&); }; static void load_config_file( const fc::path& config_ini_path, const bpo::options_description& cfg_options, @@ -84,7 +90,7 @@ static void load_config_file( const fc::path& config_ini_path, const bpo::option bpo::options_description unique_options("Graphene Witness Node"); for( const boost::shared_ptr opt : cfg_options.options() ) { - boost::shared_ptr od = dedup.next(opt); + const boost::shared_ptr od = dedup.next(opt); if( !od ) continue; unique_options.add( od ); } @@ -106,6 +112,13 @@ static void load_config_file( const fc::path& config_ini_path, const bpo::option } } +const boost::shared_ptr new_option_description( const std::string& name, const bpo::value_semantic* value, const std::string& description ) +{ + bpo::options_description helper(""); + helper.add_options()( name.c_str(), value, description.c_str() ); + return helper.options()[0]; +} + static void create_new_config_file( const fc::path& config_ini_path, const fc::path& data_dir, const bpo::options_description& cfg_options ) { @@ -113,11 +126,19 @@ static void create_new_config_file( const fc::path& config_ini_path, const fc::p if( !fc::exists(data_dir) ) fc::create_directories(data_dir); - deduplicator dedup; + auto modify_option_defaults = [](const boost::shared_ptr& o) -> const boost::shared_ptr { + const std::string& name = o->long_name(); + if( name == "partial-operations" ) + return new_option_description( name, bpo::value()->default_value(true), o->description() ); + if( name == "max-ops-per-account" ) + return new_option_description( name, bpo::value()->default_value(1000), o->description() ); + return o; + }; + deduplicator dedup(modify_option_defaults); std::ofstream out_cfg(config_ini_path.preferred_string()); for( const boost::shared_ptr opt : cfg_options.options() ) { - boost::shared_ptr od = dedup.next(opt); + const boost::shared_ptr od = dedup.next(opt); if( !od ) continue; if( !od->description().empty() )