nvharikrishna commented on code in PR #163:
URL: https://github.com/apache/cassandra-sidecar/pull/163#discussion_r1895389139
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -92,9 +112,13 @@ public InstanceConfigurationImpl(int id,
this.id = id;
this.host = host;
this.port = port;
- this.dataDirs = Collections.unmodifiableList(dataDirs);
+ this.dataDirs = dataDirs == null ? null :
Collections.unmodifiableList(dataDirs);
Review Comment:
> when would data dirs be null (or empty)? Maybe this needs to be validated
instead of accepting null
It is set to null if the data_dirs is not configured in sidecar.yaml. I was
not sure why it was allowed to be null earlier. So did not change the behavior.
I can make changes now though.
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -39,6 +39,10 @@ class InstanceMetadataImplTest
private static final String DATA_DIR_2 = "test/data/data2";
private static final String CDC_DIR = "cdc_dir";
private static final String STAGING_DIR = "staging_dir";
+ private static final String COMMITLOG_DIR = "commitlog";
Review Comment:
>so it would be incorrect to add _dir.
Wondering why CDC_DIR has "_dir" postfix. As per cassandra.yaml,
cdc_raw_directory is `cdc_raw`.
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
Review Comment:
Followed the footsteps of existing code. I would be happy to adopt
`cassandra_home_dir` as part of this pr.
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
Review Comment:
> Ideally I think we want to only have to specify the cassandra_home_dir ,
and optionally specify Cassandra directories. The exception are data directories
As per the documentation comment in
[cassandra.yaml](https://github.com/apache/cassandra/blob/trunk/conf/cassandra.yaml#L398),
the default data directory is`$CASSANDRA_HOME/data/data`. Is there any reason
to take exception for data directories?
--
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]
