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: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org