yifan-c commented on code in PR #138:
URL: https://github.com/apache/cassandra-sidecar/pull/138#discussion_r1792510963
##########
server/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java:
##########
@@ -439,6 +457,14 @@ void
validateCassandraInputValidationConfigurationFromYaml(CassandraInputValidat
assertThat(config.allowedPatternForRestrictedComponentName()).isEqualTo("[a-zA-Z0-9_-]+(.db|TOC.txt)");
}
+ void
validateVertxFilesystemOptionsConfiguration(VertxFilesystemOptionsConfiguration
config)
Review Comment:
maybe `validateVertxFilesystemOptionsClasspathResolvingDisabled`?
##########
server/src/main/dist/conf/sidecar.yaml:
##########
@@ -68,6 +68,9 @@ sidecar:
tcp_keep_alive: false
accept_backlog: 1024
server_verticle_instances: 1
+ vertx_filesystem_options:
+ class_path_resolving_enabled: false
+ file_caching_enabled: false
Review Comment:
nit: how about having the structure `vertx -> filesystem_options` to be
future-proof? In case, there are other vertx configurations to be exposed via
yaml.
##########
server/src/main/java/org/apache/cassandra/sidecar/server/MainModule.java:
##########
@@ -162,7 +164,25 @@ public Vertx vertx(SidecarConfiguration
sidecarConfiguration, MetricRegistryFact
// Monitor all V1 endpoints.
// Additional filtering is done by
configuring yaml fields 'metrics.include|exclude'
.addMonitoredHttpServerRoute(serverRouteMatch);
- return Vertx.vertx(new
VertxOptions().setMetricsOptions(dropwizardMetricsOptions));
+
+ FileSystemOptions fsOptions;
+ VertxFilesystemOptionsConfiguration configuredFSOptions =
sidecarConfiguration.serviceConfiguration()
+
.vertxFilesystemOptionsConfiguration();
+ if (configuredFSOptions != null)
+ {
+ fsOptions = new FileSystemOptions()
+
.setClassPathResolvingEnabled(configuredFSOptions.classPathResolvingEnabled())
+ .setFileCacheDir(configuredFSOptions.fileCacheDir())
+
.setFileCachingEnabled(configuredFSOptions.fileCachingEnabled());
+ }
+ else
+ {
+ fsOptions = new FileSystemOptions();
+ }
+
+ VertxOptions vertxOptions = new
VertxOptions().setMetricsOptions(dropwizardMetricsOptions)
+
.setFileSystemOptions(fsOptions);
+ return Vertx.vertx(vertxOptions);
Review Comment:
nit: this is more succinct.
```java
VertxOptions vertxOptions = new
VertxOptions().setMetricsOptions(dropwizardMetricsOptions);
VertxFilesystemOptionsConfiguration configuredFSOptions =
sidecarConfiguration.serviceConfiguration()
.vertxFilesystemOptionsConfiguration();
if (configuredFSOptions != null)
{
FileSystemOptions fsOptions = new FileSystemOptions()
.setClassPathResolvingEnabled(configuredFSOptions.classPathResolvingEnabled())
.setFileCacheDir(configuredFSOptions.fileCacheDir())
.setFileCachingEnabled(configuredFSOptions.fileCachingEnabled());
vertxOptions.setFileSystemOptions(fsOptions);
}
return Vertx.vertx(vertxOptions);
```
##########
server/src/main/java/org/apache/cassandra/sidecar/server/MainModule.java:
##########
@@ -162,7 +164,25 @@ public Vertx vertx(SidecarConfiguration
sidecarConfiguration, MetricRegistryFact
// Monitor all V1 endpoints.
// Additional filtering is done by
configuring yaml fields 'metrics.include|exclude'
.addMonitoredHttpServerRoute(serverRouteMatch);
- return Vertx.vertx(new
VertxOptions().setMetricsOptions(dropwizardMetricsOptions));
+
+ FileSystemOptions fsOptions;
+ VertxFilesystemOptionsConfiguration configuredFSOptions =
sidecarConfiguration.serviceConfiguration()
+
.vertxFilesystemOptionsConfiguration();
Review Comment:
Can `vertxFilesystemOptionsConfiguration` return null? It does not according
to the implementation.
If it can, it would be good to add `@Nullable` to the signature.
##########
server/src/test/resources/config/sidecar_multiple_instances.yaml:
##########
@@ -71,6 +71,9 @@ sidecar:
tcp_keep_alive: false
accept_backlog: 1024
server_verticle_instances: 1
+ vertx_filesystem_options:
+ class_path_resolving_enabled: false
Review Comment:
I thought `classpath` is commonly recognized as a single word :p
https://en.wikipedia.org/wiki/Classpath
--
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]
