frankgh commented on code in PR #14:
URL:
https://github.com/apache/cassandra-analytics/pull/14#discussion_r1326688112
##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/clients/Sidecar.java:
##########
@@ -129,16 +130,15 @@ public static SidecarClient from(SidecarInstancesProvider
sidecarInstancesProvid
.ssl(conf.hasKeystoreAndKeystorePassword())
.build();
- SidecarConfig sidecarConfig = new SidecarConfig.Builder()
- .maxRetries(5)
- .retryDelayMillis(200)
- .maxRetryDelayMillis(500)
- .build();
-
- return buildClient(sidecarConfig, vertx, httpClientConfig,
sidecarInstancesProvider);
+ SidecarClientConfig sidecarConfig = SidecarClientConfigImpl.builder()
+
.maxRetries(5)
+
.retryDelayMillis(200)
+
.maxRetryDelayMillis(500)
+
.build();
+ return buildClient((SidecarClientConfigImpl) sidecarConfig, vertx,
httpClientConfig, sidecarInstancesProvider);
}
- public static SidecarClient buildClient(SidecarConfig sidecarConfig,
+ public static SidecarClient buildClient(SidecarClientConfigImpl
sidecarConfig,
Review Comment:
Can we use the interface here instead?
```suggestion
public static SidecarClient buildClient(SidecarClientConfig
sidecarConfig,
```
##########
build.gradle:
##########
@@ -150,8 +166,15 @@ subprojects {
repositories {
mavenCentral()
+ mavenLocal {
Review Comment:
indentation.
##########
scripts/build-dependencies.sh:
##########
@@ -0,0 +1,23 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; )
+
+${SCRIPT_DIR}/build-dtest-jars.sh
+${SCRIPT_DIR}/build-sidecar.sh
Review Comment:
can we add a flag to conditionally build dtest-jars and/or sidecar?
##########
gradle.properties:
##########
@@ -29,5 +29,6 @@ jnaVersion=5.9.0
scala=2.12
spark=3
vertxVersion=4.2.1
+sidecarVersion=1.0.0-analytics
Review Comment:
this is already part of profiles, we shouldn't have it here
##########
build.gradle:
##########
@@ -68,9 +77,10 @@ tasks.idea.dependsOn(tasks.copyInspections)
tasks.register('buildIgnoreRatList', Exec) {
description 'Builds a list of ignored files for the rat task from the
unversioned git files'
- commandLine 'bash', '-c', 'git clean --force -d --dry-run -x | cut -c 14-'
+ commandLine 'bash', '-c', 'git clean --force -d --dry-run -x | grep -v
"Would skip" | cut -c 14- && ' +
+ 'git clean --force -d --dry-run -x | grep "Would skip" | cut -c
23-\n'
doFirst {
- standardOutput new FileOutputStream("${buildDir}/.rat-excludes.txt")
+ standardOutput new FileOutputStream(".rat-excludes.txt")
Review Comment:
why not keep it in the build dir? It is useful to debug, because these
resources are copied in CI to resources
##########
cassandra-bridge/src/main/java/org/apache/cassandra/bridge/CassandraVersion.java:
##########
@@ -29,7 +29,9 @@ public enum CassandraVersion
{
THREEZERO(30, "3.0", "three-zero"),
FOURZERO(40, "4.0", "four-zero"),
- FOURONE(41, "4.1", "four-zero");
+ FOURONE(41, "4.1", "four-zero"),
+ FIVEZERO(50, "5.0", "four-zero"),
Review Comment:
hmm, this feels like it's outside of the scope of the project. I also feel
that the four-zero implementation won't be sufficient to support 5.0, so I
would prefer to have a proper five-zero implementation.
--
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]
