sk0x50 commented on code in PR #961:
URL: https://github.com/apache/ignite-3/pull/961#discussion_r942653841
##########
modules/cli/src/main/java/org/apache/ignite/cli/deprecated/builtins/node/NodeManager.java:
##########
@@ -136,6 +137,9 @@ public RunningNode start(
if (srvCfg != null) {
cmdArgs.add("--config");
cmdArgs.add(srvCfg.toAbsolutePath().toString());
+ } else if (srvCfgStr != null) {
+ cmdArgs.add("--configStr");
Review Comment:
`--configStr` does not look descriptive to be honest :) By the way, do we
use camel case for commands? Perhaps it should be named `config-str`, for
instance see at `work-dir`.
##########
modules/cli/src/main/java/org/apache/ignite/cli/deprecated/spec/NodeCommandSpec.java:
##########
@@ -102,12 +130,32 @@ public Integer call() {
out.println(tbl);
return 0;
}
+
+ private Path getConfigPath() {
+ return configOptions != null ? configOptions.configPath : null;
+ }
+
+ private String getConfigStr() {
Review Comment:
Well, looks like these parameters (port, rest-port etc) cannot be provided
with a configuration file. Right?
It seems to me, that ability to start two nodes with the same configuration
file and overridden value of `port` would be very convenient. I mean the
following:
`node start nodeName --config \path_to_configuration_file --port 12345`
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
