nvharikrishna commented on code in PR #163:
URL: https://github.com/apache/cassandra-sidecar/pull/163#discussion_r1895908074
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -21,80 +21,63 @@
import java.util.Collections;
import java.util.List;
+import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.cassandra.sidecar.config.InstanceConfiguration;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
/**
* Encapsulates the basic configuration needed to connect to a single
Cassandra instance
*/
public class InstanceConfigurationImpl implements InstanceConfiguration
{
- @JsonProperty("id")
protected final int id;
-
- @JsonProperty("host")
protected final String host;
-
- @JsonProperty("port")
protected final int port;
-
- @JsonProperty("data_dirs")
+ protected final String cassandraHomeDir;
protected final List<String> dataDirs;
-
- @JsonProperty("staging_dir")
protected final String stagingDir;
-
- @JsonProperty("cdc_dir")
protected final String cdcDir;
-
- @JsonProperty("jmx_host")
+ protected final String commitlogDir;
+ protected final String hintsDir;
+ protected final String savedCachesDir;
+ protected final String localSystemDataFileDir;
protected final String jmxHost;
-
- @JsonProperty("jmx_port")
protected final int jmxPort;
-
- @JsonProperty("jmx_ssl_enabled")
protected final boolean jmxSslEnabled;
-
- @JsonProperty("jmx_role")
protected final String jmxRole;
-
- @JsonProperty("jmx_role_password")
protected final String jmxRolePassword;
- public InstanceConfigurationImpl()
Review Comment:
Removed this in order to reduce the confusion with what can be null and what
cannot be null.
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -21,80 +21,63 @@
import java.util.Collections;
import java.util.List;
+import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.cassandra.sidecar.config.InstanceConfiguration;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
/**
* Encapsulates the basic configuration needed to connect to a single
Cassandra instance
*/
public class InstanceConfigurationImpl implements InstanceConfiguration
{
- @JsonProperty("id")
Review Comment:
Using constructor for this.
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -52,15 +65,42 @@ protected InstanceMetadataImpl(Builder builder)
id = builder.id;
host = builder.host;
port = builder.port;
+ cassandraHomeDir =
FileUtils.maybeResolveHomeDirectory(builder.cassandraHomeDir);
+ Path cassandraHomeDirPath = cassandraHomeDir == null ? null :
Paths.get(cassandraHomeDir);
Review Comment:
Okay with having cassandraHomeDir as a mandatory configuration? If yes, then
I can push changes to make it mandatory. Assuming it is not mandatory for now.
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -21,80 +21,63 @@
import java.util.Collections;
import java.util.List;
+import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.cassandra.sidecar.config.InstanceConfiguration;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
/**
* Encapsulates the basic configuration needed to connect to a single
Cassandra instance
*/
public class InstanceConfigurationImpl implements InstanceConfiguration
{
- @JsonProperty("id")
protected final int id;
-
- @JsonProperty("host")
protected final String host;
-
- @JsonProperty("port")
protected final int port;
-
- @JsonProperty("data_dirs")
+ protected final String cassandraHomeDir;
protected final List<String> dataDirs;
-
- @JsonProperty("staging_dir")
protected final String stagingDir;
-
- @JsonProperty("cdc_dir")
protected final String cdcDir;
-
- @JsonProperty("jmx_host")
+ protected final String commitlogDir;
+ protected final String hintsDir;
+ protected final String savedCachesDir;
+ protected final String localSystemDataFileDir;
protected final String jmxHost;
-
- @JsonProperty("jmx_port")
protected final int jmxPort;
-
- @JsonProperty("jmx_ssl_enabled")
protected final boolean jmxSslEnabled;
-
- @JsonProperty("jmx_role")
protected final String jmxRole;
-
- @JsonProperty("jmx_role_password")
protected final String jmxRolePassword;
- public InstanceConfigurationImpl()
- {
- this.id = 0;
- this.host = null;
- this.port = 9042;
- this.dataDirs = null;
- this.stagingDir = null;
- this.cdcDir = null;
- this.jmxHost = null;
- this.jmxPort = 0;
- this.jmxSslEnabled = false;
- this.jmxRole = null;
- this.jmxRolePassword = null;
- }
-
- public InstanceConfigurationImpl(int id,
- String host,
- int port,
- List<String> dataDirs,
- String stagingDir,
- String cdcDir,
- String jmxHost,
- int jmxPort,
- boolean jmxSslEnabled,
- String jmxRole,
- String jmxRolePassword)
+ @JsonCreator
+ public InstanceConfigurationImpl(@JsonProperty("id") int id,
+ @NotNull @JsonProperty("host") String
host,
Review Comment:
@frankgh I guessing a bit for `NotNull` and `Nullable` of these fields.
Please suggest changes if my interpretation is incorrect.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
