You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "nvharikrishna (via GitHub)" <gi...@apache.org> on 2026/02/03 08:26:34 UTC
[PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
nvharikrishna opened a new pull request, #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312
Added support for configuring the storage_port in Cassandra instance metadata and configuration. This enhancement exposes the storage port through the InstanceMetadata and InstanceConfiguration interfaces, allowing Sidecar to be aware of and utilize this configuration parameter.
For CASSSIDECAR-399
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "skoppu22 (via GitHub)" <gi...@apache.org>.
skoppu22 commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2768497835
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -237,6 +254,7 @@ public static class Builder implements DataObjectBuilder<Builder, InstanceMetada
protected String host;
protected String ipAddress;
protected int port;
+ protected int storagePort = DEFAULT_STORAGE_PORT;
Review Comment:
Thanks for details.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2789793719
##########
server/src/main/java/org/apache/cassandra/sidecar/modules/ConfigurationModule.java:
##########
@@ -240,21 +240,29 @@ private static InstanceMetadata buildInstanceMetadata(Vertx vertx,
host,
port,
new InstanceHealthMetrics(instanceSpecificRegistry));
- return InstanceMetadataImpl.builder()
- .id(cassandraInstance.id())
- .host(host, dnsResolver)
- .port(port)
- .storageDir(cassandraInstance.storageDir())
- .dataDirs(cassandraInstance.dataDirs())
- .stagingDir(cassandraInstance.stagingDir())
- .cdcDir(cassandraInstance.cdcDir())
- .commitlogDir(cassandraInstance.commitlogDir())
- .hintsDir(cassandraInstance.hintsDir())
- .savedCachesDir(cassandraInstance.savedCachesDir())
- .localSystemDataFileDir(cassandraInstance.localSystemDataFileDir())
- .lifecycleOptions(cassandraInstance.lifecycleOptions())
- .delegate(delegate)
- .metricRegistry(instanceSpecificRegistry)
- .build();
+ InstanceMetadataImpl.Builder builder = InstanceMetadataImpl.builder()
+ .id(cassandraInstance.id())
+ .host(host, dnsResolver)
+ .port(port);
+
+ // Only set storagePort if explicitly configured, otherwise use builder default
+ Integer configuredStoragePort = cassandraInstance.storagePort();
+ if (configuredStoragePort != null)
+ {
+ builder.storagePort(configuredStoragePort);
+ }
Review Comment:
is this check still applicable? `configuredStoragePort` should never be null
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "nvharikrishna (via GitHub)" <gi...@apache.org>.
nvharikrishna commented on PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#issuecomment-3880054725
FYI, just now pushed a changes to initialize default storage port in InstanceConfigurationImpl class (instead of InstanceMetadataImpl) to inline with other configuration implementations.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2789721852
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -33,9 +33,15 @@
*/
public class InstanceConfigurationImpl implements InstanceConfiguration
{
+ /**
+ * Default storage port for Cassandra inter-node communication.
+ */
+ public static final int DEFAULT_STORAGE_PORT = 7000;
+
protected final int id;
protected final String host;
protected final int port;
+ protected final Integer storagePort;
Review Comment:
should this be int now that it's always provided?
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -119,6 +127,17 @@ public int port()
return port;
}
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ @JsonProperty("storage_port")
+ @Nullable
+ public Integer storagePort()
Review Comment:
can we make this int as well? And the interface should also be a primitive
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "nvharikrishna (via GitHub)" <gi...@apache.org>.
nvharikrishna commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2789860785
##########
server/src/main/java/org/apache/cassandra/sidecar/modules/ConfigurationModule.java:
##########
@@ -240,21 +240,29 @@ private static InstanceMetadata buildInstanceMetadata(Vertx vertx,
host,
port,
new InstanceHealthMetrics(instanceSpecificRegistry));
- return InstanceMetadataImpl.builder()
- .id(cassandraInstance.id())
- .host(host, dnsResolver)
- .port(port)
- .storageDir(cassandraInstance.storageDir())
- .dataDirs(cassandraInstance.dataDirs())
- .stagingDir(cassandraInstance.stagingDir())
- .cdcDir(cassandraInstance.cdcDir())
- .commitlogDir(cassandraInstance.commitlogDir())
- .hintsDir(cassandraInstance.hintsDir())
- .savedCachesDir(cassandraInstance.savedCachesDir())
- .localSystemDataFileDir(cassandraInstance.localSystemDataFileDir())
- .lifecycleOptions(cassandraInstance.lifecycleOptions())
- .delegate(delegate)
- .metricRegistry(instanceSpecificRegistry)
- .build();
+ InstanceMetadataImpl.Builder builder = InstanceMetadataImpl.builder()
+ .id(cassandraInstance.id())
+ .host(host, dnsResolver)
+ .port(port);
+
+ // Only set storagePort if explicitly configured, otherwise use builder default
+ Integer configuredStoragePort = cassandraInstance.storagePort();
+ if (configuredStoragePort != null)
+ {
+ builder.storagePort(configuredStoragePort);
+ }
Review Comment:
Right, not applicable as storagePort is a primitive now. Pushed the changes.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "nvharikrishna (via GitHub)" <gi...@apache.org>.
nvharikrishna commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2763956393
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ for (int customStoragePort : new int[]{ 1, 65535, 7005 })
+ {
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .storagePort(customStoragePort)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(customStoragePort);
+ }
+ }
+
+ @Test
+ void testInvalidStoragePort()
Review Comment:
done
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ for (int customStoragePort : new int[]{ 1, 65535, 7005 })
+ {
Review Comment:
done
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
Review Comment:
done
##########
server/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java:
##########
@@ -588,6 +588,41 @@ void testRepairConfiguration() throws IOException
assertThat(repairConfig.repairPollInterval()).isEqualTo(MillisecondBoundConfiguration.parse("500ms"));
}
+ @Test
+ void testStoragePortConfiguration() throws IOException
+ {
+ // Test with explicit storage_port
+ String yamlWithStoragePort = "cassandra_instances:\n" +
+ " - id: 1\n" +
+ " host: localhost1\n" +
+ " port: 9042\n" +
+ " storage_port: 7001\n" +
+ " storage_dir: /var/lib/cassandra\n" +
+ " staging_dir: /var/lib/cassandra/staging\n" +
+ " jmx_host: 127.0.0.1\n" +
+ " jmx_port: 7199\n" +
+ " jmx_ssl_enabled: false";
+ SidecarConfigurationImpl configWithStoragePort = SidecarConfigurationImpl.fromYamlString(yamlWithStoragePort);
+ assertThat(configWithStoragePort.cassandraInstances()).isNotNull().hasSize(1);
+ InstanceConfiguration instance1 = configWithStoragePort.cassandraInstances().get(0);
+ assertThat(instance1.storagePort()).isEqualTo(7001);
+
+ // Test without storage_port (should return null)
+ String yamlWithoutStoragePort = "cassandra_instances:\n" +
Review Comment:
done
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -237,6 +254,7 @@ public static class Builder implements DataObjectBuilder<Builder, InstanceMetada
protected String host;
protected String ipAddress;
protected int port;
+ protected int storagePort = DEFAULT_STORAGE_PORT;
Review Comment:
Thanks for the thoughtful review! Let me share the reasoning behind using DEFAULT_STORAGE_PORT:
1. Independence from Cassandra process availability
Goal is to let sidecar function even when Cassandra process is not running or unreachable. This is particularly important in scenarios where:
- Sidecar is started before Cassandra
- Cassandra is temporarily unavailable
- During initialization or live migration (CEP-40) when Cassandra may not yet be part of the ring
2. 7000 port - following Cassandra's default value
The DEFAULT_STORAGE_PORT = 7000 is not Sidecar's own local value but rather the default value that Cassandra itself uses (defined in cassandra.yaml and Config.java). By using this default, it is mirroring Cassandra's own behavior when the value isn't explicitly configured. This should actually help developers since it matches Cassandra's defaults. Of course, if an operator has configured a non-standard storage_port in their Cassandra instance, they should configure the same value in Sidecar's configuration.
Cassandra uses IP + port for node identification, and Sidecar needs to correctly identify nodes even when Cassandra is temporarily unreachable. Using null, -1, or error messages when we can't reach Cassandra would actually hinder operations in the above scenarios. I feel the default value simply provides a sensible fallback.
Does this reasoning make sense?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "nvharikrishna (via GitHub)" <gi...@apache.org>.
nvharikrishna commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2789760284
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -33,9 +33,15 @@
*/
public class InstanceConfigurationImpl implements InstanceConfiguration
{
+ /**
+ * Default storage port for Cassandra inter-node communication.
+ */
+ public static final int DEFAULT_STORAGE_PORT = 7000;
+
protected final int id;
protected final String host;
protected final int port;
+ protected final Integer storagePort;
Review Comment:
Makes sense, done
##########
server/src/main/java/org/apache/cassandra/sidecar/config/yaml/InstanceConfigurationImpl.java:
##########
@@ -119,6 +127,17 @@ public int port()
return port;
}
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ @JsonProperty("storage_port")
+ @Nullable
+ public Integer storagePort()
Review Comment:
done
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "skoppu22 (via GitHub)" <gi...@apache.org>.
skoppu22 commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2763382248
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -237,6 +254,7 @@ public static class Builder implements DataObjectBuilder<Builder, InstanceMetada
protected String host;
protected String ipAddress;
protected int port;
+ protected int storagePort = DEFAULT_STORAGE_PORT;
Review Comment:
I view the Sidecar as a service exposing data/config of Cassandra servers in the cluster, so developers/users can work with Cassandra accordingly. Is it OK to use Sidecar's local DEFAULT_STORAGE_PORT value here? Why don't we just give whatever we got from the Cassandra server? If we get null/nothing from the server, we can either set something like -1 or error message in the Sidecar's response, somehow indicating to the caller that Sidecar unable to get this value from the server? My perspective is, using Sidecar's own local values may mislead developers/users while debugging an issue with Cassandra.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2761281555
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ for (int customStoragePort : new int[]{ 1, 65535, 7005 })
+ {
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .storagePort(customStoragePort)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(customStoragePort);
+ }
+ }
+
+ @Test
+ void testInvalidStoragePort()
Review Comment:
NIT: prefer parameterized test
```suggestion
@ParameterizedTest(name = "{index} => invalid storagePort={0}")
@ValueSource(ints = { 0, -1, 65536 })
```
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
Review Comment:
NIT: prefer parameterized test
```suggestion
@ParameterizedTest(name = "{index} => storagePort={0}")
@ValueSource(ints = { 1, 65535, 7005 })
void testCustomStoragePort(int customStoragePort)
```
##########
server/src/test/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImplTest.java:
##########
@@ -200,6 +200,56 @@ void testResolvesDefaultDirectories()
assertThat(metadata.savedCachesDir()).isEqualTo(rootDir + "/saved_caches");
}
+ @Test
+ void testDefaultStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ InstanceMetadata metadata = InstanceMetadataImpl.builder()
+ .id(ID)
+ .host(HOST)
+ .port(PORT)
+ .metricRegistry(METRIC_REGISTRY)
+ .storageDir(rootDir)
+ .build();
+ assertThat(metadata.storagePort()).isEqualTo(7000);
+ }
+
+ @Test
+ void testCustomStoragePort()
+ {
+ String rootDir = tempDir.toString();
+ for (int customStoragePort : new int[]{ 1, 65535, 7005 })
+ {
Review Comment:
Not needed when using a parameterized test
```suggestion
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2766007341
##########
conf/sidecar.yaml:
##########
@@ -24,6 +24,11 @@ cassandra_instances:
host: localhost1
port: 9042
+ # The storage port used for inter-node communication between Cassandra nodes in a cluster.
+ # This port is used for internal gossip protocol, data replication, and repair operations.
+ # Default: 7000
+ #storage_port: 7000
Review Comment:
Appreciate the document
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh merged PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-399 Adding storage_port to instance configuration and metadata [cassandra-sidecar]
Posted by "skoppu22 (via GitHub)" <gi...@apache.org>.
skoppu22 commented on code in PR #312:
URL: https://github.com/apache/cassandra-sidecar/pull/312#discussion_r2763404905
##########
server/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java:
##########
@@ -588,6 +588,41 @@ void testRepairConfiguration() throws IOException
assertThat(repairConfig.repairPollInterval()).isEqualTo(MillisecondBoundConfiguration.parse("500ms"));
}
+ @Test
+ void testStoragePortConfiguration() throws IOException
+ {
+ // Test with explicit storage_port
+ String yamlWithStoragePort = "cassandra_instances:\n" +
+ " - id: 1\n" +
+ " host: localhost1\n" +
+ " port: 9042\n" +
+ " storage_port: 7001\n" +
+ " storage_dir: /var/lib/cassandra\n" +
+ " staging_dir: /var/lib/cassandra/staging\n" +
+ " jmx_host: 127.0.0.1\n" +
+ " jmx_port: 7199\n" +
+ " jmx_ssl_enabled: false";
+ SidecarConfigurationImpl configWithStoragePort = SidecarConfigurationImpl.fromYamlString(yamlWithStoragePort);
+ assertThat(configWithStoragePort.cassandraInstances()).isNotNull().hasSize(1);
+ InstanceConfiguration instance1 = configWithStoragePort.cassandraInstances().get(0);
+ assertThat(instance1.storagePort()).isEqualTo(7001);
+
+ // Test without storage_port (should return null)
+ String yamlWithoutStoragePort = "cassandra_instances:\n" +
Review Comment:
Can we split this second part into a different test function, like testYamlWithoutStoragePort() etc, that will make clear to the reader that we are testing this scenario?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org