yifan-c commented on code in PR #88:
URL:
https://github.com/apache/cassandra-analytics/pull/88#discussion_r1792282366
##########
cassandra-bridge/build.gradle:
##########
@@ -40,14 +40,32 @@ publishing {
dependencies {
api(project(':cassandra-analytics-common'))
- api(project(':cassandra-analytics-spark-converter'))
- compileOnly(group: "${sparkGroupId}", name:
"spark-core_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
- compileOnly(group: "${sparkGroupId}", name:
"spark-sql_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
+ implementation "org.slf4j:slf4j-api:${slf4jApiVersion}"
+ compileOnly "com.esotericsoftware:kryo-shaded:${kryoVersion}"
+ compileOnly "com.google.guava:guava:${guavaVersion}"
+ compileOnly
"com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}"
+ compileOnly "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}"
+ compileOnly "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}"
+
+ testImplementation(project(path: ":cassandra-analytics-spark-converter"))
+ testImplementation(group: "${sparkGroupId}", name:
"spark-core_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
+ testImplementation(group: "${sparkGroupId}", name:
"spark-sql_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
testImplementation(group: 'com.fasterxml.jackson.core', name:
'jackson-databind', version: "${jacksonVersion}")
testImplementation(group: 'com.google.guava', name: 'guava', version:
'31.1-jre')
testImplementation(group: 'org.slf4j', name: 'slf4j-simple', version:
'1.7.26')
testImplementation("org.junit.jupiter:junit-jupiter-api:${project.junitVersion}")
testImplementation("org.junit.jupiter:junit-jupiter-params:${project.junitVersion}")
testImplementation("org.junit.jupiter:junit-jupiter-engine:${project.junitVersion}")
}
+
+configurations {
+ testArtifacts
+}
+task testJar(type: Jar) {
+ archiveBaseName = "${project.name}-bridge-test"
+ from sourceSets.test.output
+}
+artifacts {
+ testArtifacts testJar
+}
Review Comment:
Using test-fixture instead?
https://docs.gradle.org/current/userguide/java_testing.html#sec:java_test_fixtures
##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/data/DataLayer.java:
##########
@@ -155,6 +157,11 @@ public CassandraVersion version()
*/
public abstract CassandraBridge bridge();
+ public SparkSqlTypeConverter typeConverter()
Review Comment:
Add java doc
##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/bridge/CassandraBridgeFactory.java:
##########
@@ -75,12 +78,32 @@ public static CassandraBridge get(@NotNull
CassandraVersionFeatures features)
return get(getCassandraVersion(features));
}
+ @NotNull
+ public static SparkSqlTypeConverter getSparkSql(@NotNull
CassandraVersionFeatures features)
+ {
+ return getSparkSql(getCassandraVersion(features));
+ }
+
Review Comment:
Please locate the method together with the other `getSparkSql` overloads.
##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/bridge/CassandraBridgeFactory.java:
##########
@@ -33,12 +33,15 @@
import com.google.common.base.Preconditions;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.cassandra.spark.data.converter.SparkSqlTypeConverter;
import org.jetbrains.annotations.NotNull;
public final class CassandraBridgeFactory
{
- private static final Map<String, CassandraBridge> CASSANDRA_BRIDGES = new
ConcurrentHashMap<>(CassandraVersion.values().length);
+ private static final Map<String, Pair<CassandraBridge,
SparkSqlTypeConverter>> CASSANDRA_BRIDGES =
Review Comment:
nit: add a comment to indicate what is the key String value represents
##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/bridge/CassandraBridgeFactory.java:
##########
@@ -130,14 +153,20 @@ private static String typesResourceName(@NotNull String
label)
return jarResourceName(label, "types");
}
+ @NotNull
+ private static String sparkSqlResourceName(@NotNull String label)
+ {
+ return jarResourceName(label, "sparksql");
+ }
+
private static String jarResourceName(String... parts)
{
return "/bridges/" + String.join("-", parts) + ".jar";
}
@NotNull
@SuppressWarnings("unchecked")
- private static CassandraBridge create(@NotNull String label)
+ private static Pair<CassandraBridge, SparkSqlTypeConverter>
create(@NotNull String label)
Review Comment:
The method signature looks a bit odd. `CassandraBridgeFactory#create`
returns a pair..
How about create a dedicated data class, say `VersionedCassandraBridge`,
that wraps both `CassandraBridge` and `SparkSqlTypeConverter`? Then `create`
and `get` methods return the `VersionedCassandraBridge`. `getSparkSql` may be
removed.
```java
public class VersionedCassandraBridge
{
public final CassandraBridge cassandraBridge;
public final SparkSqlTypeConverter sparkSqlTypeConverter;
}
```
##########
cassandra-bridge/build.gradle:
##########
@@ -40,14 +40,32 @@ publishing {
dependencies {
api(project(':cassandra-analytics-common'))
- api(project(':cassandra-analytics-spark-converter'))
- compileOnly(group: "${sparkGroupId}", name:
"spark-core_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
- compileOnly(group: "${sparkGroupId}", name:
"spark-sql_${scalaMajorVersion}", version:
"${project.rootProject.sparkVersion}")
+ implementation "org.slf4j:slf4j-api:${slf4jApiVersion}"
+ compileOnly "com.esotericsoftware:kryo-shaded:${kryoVersion}"
+ compileOnly "com.google.guava:guava:${guavaVersion}"
+ compileOnly
"com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}"
+ compileOnly "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}"
+ compileOnly "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}"
Review Comment:
nit: use the same parenthesis style as the other lines. `compileOnly(...)`
--
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]
