feat: add config verify flag #1814
Conversation
PeterChen13579
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1814 +/- ##
===========================================
+ Coverage 71.40% 71.53% +0.12%
===========================================
Files 315 322 +7
Lines 13113 13235 +122
Branches 6683 6737 +54
===========================================
+ Hits 9363 9467 +104
+ Misses 1965 1945 -20
- Partials 1785 1823 +38 ☔ View full report in Codecov by Sentry. |
| @@ -47,6 +47,7 @@ CliArgs::parse(int argc, char const* argv[]) | |||
| ("conf,c", po::value<std::string>()->default_value(kDEFAULT_CONFIG_PATH), "configuration file") | |||
| ("ng-web-server,w", "Use ng-web-server") | |||
| ("migrate", po::value<std::string>(), "start migration helper") | |||
| ("verify", "Checks the validity of config values") | |||
There was a problem hiding this comment.
Maybe this flag should take path to config as well?
There was a problem hiding this comment.
I think it already does here: https://github.com/XRPLF/clio/pull/1814/files#diff-bad42112163fd471148e3dbe18fbb299652a1d323e8d0a0c1a526cba10928a2dR80
There was a problem hiding this comment.
I meant to verify config currently user should run command clio_server -c ./config.json --verify, right?
I was thinking that maybe clio_server --verify ./config.json may be better, but maybe it doesn't matter.
@godexsoft, WDYT?
| @@ -47,6 +47,7 @@ CliArgs::parse(int argc, char const* argv[]) | |||
| ("conf,c", po::value<std::string>()->default_value(kDEFAULT_CONFIG_PATH), "configuration file") | |||
| ("ng-web-server,w", "Use ng-web-server") | |||
| ("migrate", po::value<std::string>(), "start migration helper") | |||
| ("verify", "Checks the validity of config values") | |||
There was a problem hiding this comment.
I meant to verify config currently user should run command clio_server -c ./config.json --verify, right?
I was thinking that maybe clio_server --verify ./config.json may be better, but maybe it doesn't matter.
@godexsoft, WDYT?
| return EXIT_FAILURE; | ||
| [](app::CliArgs::Action::VerifyConfig const& verify) { | ||
| if (app::verifyConfig(verify.configPath)) { | ||
| std::cout << "Config is correct" << "\n"; |
There was a problem hiding this comment.
Probably better to print config path here, something like
Config './config.json` is correct
|
|
||
| auto const json = ConfigFileJson::makeConfigFileJson(configPath); | ||
| if (!json.has_value()) { | ||
| std::cerr << json.error().error << std::endl; |
There was a problem hiding this comment.
Maybe a bit more verbose:
std::cerr << "Error parsing json from config: " << configPath << "\n" << json.error().error << std::endl;| } | ||
| auto const errors = gClioConfig.parse(json.value()); | ||
| if (errors.has_value()) { | ||
| for (auto const& err : errors.value()) |
There was a problem hiding this comment.
And the same here:
std::cerr << "Issues found in provided config '" << configPath << "':\n" ;| // used to Verify Config test | ||
| static constexpr auto kVALID_JSON_DATA = R"JSON({ | ||
| "server": { | ||
| "ip": "0.0.0.0", | ||
| "port": 51233 | ||
| } | ||
| })JSON"; |
There was a problem hiding this comment.
This should be in the file where it is used.
This reverts commit f1698c5.
|
@kuznetsss The verify flag can work both with and without the |
fixes #1806