Enable -Werror and fix all Scala compilation warnings - #1385
Open
dkropachev wants to merge 62 commits into
Open
Conversation
- Use `com.scylladb` as organization name - Use `spark-scylladb-connector` as artifact name - Explain the reasons for the fork in the README
Some of the github actions are super old. Let's have all of em updated.
Upgrade github actions
Introduce Makefile with targets for: - Version resolution (Scala, Cassandra, Scylla) using get-version tool - CCM installation for both Cassandra and Scylla - Database image pre-download - Integration test execution with proper environment setup - CI matrix generation for parallel test runs
- Add reusable integration-tests.yml workflow with dynamic matrix - Simplify main.yml to call reusable workflow with configurable inputs - Add concurrency group to cancel stale runs - Update action versions (checkout@v6, setup-java@v5, junit-report@v6) - Add support for database version aliases (LATEST, LTS-LATEST, etc.) - Enable CCM caching for faster CI runs
- Add scyllaEnabled config option to CcmConfig - Handle Scylla version resolution from CCM with caching - Return V3_10 as Cassandra version equivalent for Scylla - Skip refreshsizeestimates nodetool command on Scylla - Adjust CCM cluster creation for Scylla (--scylla flag) - Skip Scylla-unsupported JVM args and wait options - Enable UDF experimental features for Scylla clusters - Add CCM_IS_SCYLLA environment variable support in Testing.scala - Add Java 9+ module system --add-opens options for Spark
SparkCassandraITFlatSpecBase: - Add isScylla check method - Add notScylla(issue) helper to skip tests with issue reference - Add scyllaOnly helper for Scylla-specific tests - Add withRetry helper for transient connection issues Fixtures: - Add ScyllaFixture trait with assumeNotScylla/assumeScylla - Add ScyllaAwareCluster trait for Scylla-compatible tests - Configure PasswordAuthenticator for Scylla in AuthCluster
Skip or adapt tests for Scylla behavioral differences: - SSL/Auth tests: Skip due to different config format - SchemaSpec: Filter out index backing tables (Scylla exposes them) - CassandraCatalogTableSpec: Normalize class names, skip clustering order tests - CassandraRDDSpec: Add retry logic for bulk inserts - ConnectorMetricsSpec: Add tolerance for metrics race conditions - RDDSpec: Handle Scylla's silent null partition key behavior - CassandraDataFrameSpec: Skip clustering order test - CassandraSQLSpec: Skip token+partition key restriction tests - TableWriterSpec: Skip list prepend test (batching issue) Issue references added for skipped tests to track Scylla limitations.
Configure scalafix for code style enforcement with Makefile targets (lint, lint-fix) and GitHub Actions CI integration.
Apply automated fixes for linting rules across connector and driver modules.
Cache ~/.m2/repository in both lint and test jobs to speed up CI builds by avoiding repeated Maven dependency downloads.
Add Makefile and Scylla test support
Replace org.apache.cassandra driver 4.18.1 with com.scylladb driver 4.19.0.4. The ScyllaDB driver is API-compatible and provides better integration with ScyllaDB. Changes: - Update Dependencies.scala to use com.scylladb groupId - Rename CassandraJavaDriver to ScyllaJavaDriver in Versions.scala - Update driver exclusion rule for new groupId - Remove obsolete MaxPermSize JVM option from sbt launcher (Java 9+)
Switch from Apache Cassandra driver to ScyllaDB Java driver
Fixes #29 - SSL configuration format differs between Cassandra and Scylla. Scylla uses PEM-format certificates (certificate/keyfile) while Cassandra uses JKS keystores. This change: - Add `withSslPem` and `withSslAuthPem` methods to CcmConfig for Scylla's PEM-based SSL configuration - Add script to generate TLS certificates with proper Subject Alternative Names for test IP addresses (127.x.x.x) - Move TLS resources to tls/ subdirectory and generate them dynamically - Update SSLCluster and AuthCluster fixtures to use PEM format when running against Scylla - Enable SSL tests for Scylla by removing assumeNotScylla calls - Change CassandraSSLClientAuthConnectorSpec to use AuthCluster for proper client authentication testing TLS certificates are now generated by `make generate-test-certs` and are automatically created when running integration tests.
This test inserts 1 million rows (64MB of data) which overwhelms the Scylla cluster in CI environments. The single-node Scylla cluster becomes unresponsive under this heavy load. Changes: - Add SeparateJVM marker to isolate the test in its own cluster - Add ScyllaFixture to enable Scylla-aware test behavior - Skip data setup in beforeClass when running against Scylla - Add assumeNotScylla to each individual test to properly mark them as canceled (avoiding suite ABORTED status) - Track the issue as #32 The test continues to run for Cassandra, which handles the load better in CI environments. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add Scylla SSL support with PEM certificates
Enable SSL/Auth tests for Scylla
- Update integration-tests.yml: Java 8/11 → Java 17 - Update release.yml: Java 8 → Java 17 - Remove 2-PRIOR Scala version from CI defaults (only 2.13 supported) - Upgrade Mockito from 1.10.19 to 4.11.0 (Java 17+ compatible) - Switch from mockito-all to mockito-core + mockito-inline - Fix Mockito API changes: Matchers → ArgumentMatchers, anyString() → any[String] (null matching behavior change)
Switch CI pipeline to Java 17+ and upgrade Mockito
- Upgrade Apache Spark from 3.5.0 to 4.0.0 - Drop Scala 2.12 support, keep only Scala 2.13 - Target Java 17+ (update javac/scalac options) - Upgrade sbt-assembly from 0.14.10 to 2.2.0 - Fix Spark 4.x internal API breaking changes: - Strategy → SparkStrategy - Column.expr → classic.ColumnConversions.expression() - Dataset.apply → classic.Dataset - Predicate.create → InterpretedPredicate with BindReferences - AnalysisException constructor (errorClass + messageParameters) - NoSuchNamespaceException/NoSuchTableException constructors - SecurityManager removed from CassandraSink - sqlContext.conf → sessionState.conf - Merge Scala 2.13-specific sources into main source dirs - Remove Scala 2.12-specific source directories - Remove Jetty dependencies (handled by Spark 4 internally)
- Add CassandraAnalysisException subclass to access protected constructor (Spark 4.x requires registered errorClass for public constructors) - Replace custom errorClass strings with plain message AnalysisException - Update CassandraDataFrameSpec table-not-found assertions for new Spark 4.x error message format (TABLE_OR_VIEW_NOT_FOUND)
In Spark 4.x, queryExecution.logical returns the unanalyzed plan (containing UnresolvedDataSource) rather than the analyzed plan with DataSourceV2Relation. The setDirectJoin transform never found any DataSourceV2Relation to modify. Fix by using queryExecution.analyzed to get the resolved plan containing DataSourceV2Relation nodes. Also update CassandraTable.catalogConf to ensure the setting propagates through newScanBuilder. Additionally, fix consolidateConfs to normalize userOptions keys to lowercase since CaseInsensitiveStringMap.asScala.toMap produces a case-sensitive map with original-case keys.
Migrate to Apache Spark 4.0 and drop Scala 2.12 support
When multiple list mutations to the same row are sent in a BatchStatement, the driver assigns a single client-side timestamp to the entire batch. Scylla treats mutations with identical timestamps as conflicting writes and keeps only one, causing silent data loss. Fix by adding USING TIMESTAMP with a unique, monotonically incrementing microsecond value to each UPDATE statement. This ensures every list mutation within a batch gets a distinct timestamp at the CQL level, which the binary protocol preserves per-statement (unlike the batch-level query timestamp). Also adds USING TIMESTAMP support to the UPDATE query path for user-configured static and per-row timestamps, which was previously only implemented for INSERT statements. Fixes: #26
Fix list append/prepend losing elements when batched on Scylla
Rewrite release workflow to match java-driver-4.x release pattern: - Switch from GitHub release event to workflow_dispatch with inputs (dry-run, skip-tests, target-tag) - Add Makefile targets: release-prepare, release, release-dry-run, checkout-one-commit-before with environment validation - Add version.sbt for version tracking (analogous to pom.xml version) - Configure git user as ScyllaDB Promoter - Upload release logs as artifacts - Push tags after successful release
The 'set ThisBuild / test := {}' command was being split by the shell
into separate arguments. Use eval with quoted strings to preserve it
as a single SBT command.
sbt-ci-release auto-imports GPG key from PGP_SECRET, but since we call +publishSigned directly, we need to import it explicitly via gpg --import. Also simplify Makefile by removing unused env validation and variables.
Sonatype sunset the legacy OSSRH endpoint (oss.sonatype.org) on 2025-06-30. Migrate to the Central Portal (central.sonatype.com): - Upgrade sbt from 1.10.0 to 1.11.7 (built-in Central Portal support) - Upgrade sbt-ci-release from 1.6.1 to 1.11.2 - Configure publishTo with localStaging for releases, Central Portal for snapshots - Replace sonatypeBundleRelease with sonaUpload + sonaRelease commands - Dry-run uploads to Central Portal without auto-releasing (can verify and drop manually), matching java-driver-4.x behavior - Use existing repo secrets (PGP_SECRET, PGP_PASSPHRASE) for GPG and SONATYPE_TOKEN_USERNAME/PASSWORD for Central Portal tokens
Add release CI/CD pipeline with manual workflow dispatch
The AsyncExecutor retried AllNodesFailedException only when wrapping BusyConnectionException, but Scylla reports NodeUnavailableException under connection pool pressure — causing immediate failure instead of retry. This was the root cause of flaky CassandraRDDSpec on Scylla CI. Changes: - Add NodeUnavailableException to the retryable error conditions - Add exponential backoff (100ms base, 5s cap) to all retries - Add retry count limit (default 10) to prevent unbounded retry loops - Fix getAllErrors() iteration to handle Map<Node, List<Throwable>> Fixes #35 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix AsyncExecutor retry for NodeUnavailableException
…leException When the ScyllaDB driver throws AllNodesFailedException containing a non-serializable ShardingInfo reference, ScalaTest's forked test reporter fails to serialize the SuiteAborted event, causing CI exit code 2 despite all tests passing. The base class SparkCassandraITSpecBase already wraps beforeClass/afterAll/ withFixture with wrapUnserializableExceptions(), but 8 test specs had DB initialization running directly in the class constructor body, bypassing this protection. Move that initialization into beforeClass and convert dependent vals to lazy vals so they evaluate after setup completes. Fixes #38 See also: scylladb/java-driver#805
GitHub runners have ~7GB RAM. The auto-detection in Testing.scala calculates (7168 - 1550) / 2560 = 2 parallel tasks, so bumping from 1 to 2 matches what the runner can handle and should cut integration test time significantly.
CI: Increase test parallelism from 1 to 2
…allelism - Add a shared compile job that caches compiled classes for reuse by lint and test jobs, avoiding redundant compilation across matrix entries - Increase TEST_PARALLEL_TASKS from 1 to 2 for integration tests - Add `make compile` target for compiling all modules including test sources - Lint job now depends on compile job and restores cached classes Closes #42, Closes #43
CI: Speed up integration tests with compile caching and parallelism
…HELL Without .SHELLFLAGS, the default is just -c, meaning errors in multi-line recipes are silently ignored under .ONESHELL. Adding -ec ensures the shell exits on the first error.
- Add || true to find commands that fail when version cache files don't exist - Move ccm --help into if-condition so non-zero exit doesn't abort the shell
With .ONESHELL, recipe lines are passed as a single script to the shell, making backslash continuations unnecessary. Use && at end of line for conditional chains and join eval redirections.
Makefile: add .SHELLFLAGS for error handling with .ONESHELL
…retry.maxRetries Add spark.cassandra.query.retry.maxRetries config parameter (default: 10) to CassandraConnectorConf, allowing users to tune the retry limit for transient errors (NodeUnavailableException, BusyConnectionException, OverloadedException). Thread the config through QueryExecutor to all call sites: TableWriter, CassandraJoinRDD, CassandraLeftJoinRDD, CassandraInJoinReaderFactory.
Make AsyncExecutor retry backoff configurable
…issue-38 Fix NotSerializableException: ShardingInfo crashes test reporter on Scylla CI
…adata The fetchClusteringColumns method was creating ClusteringColumn(index) without mapping the ClusteringOrder from the Java driver metadata, causing all clustering columns to default to ASC regardless of their actual ordering. Closes #25
Scylla now correctly reports DESC clustering order in system_schema.columns. Verified on Scylla 2025.2.5 using the Python driver that clustering_order=DESC is properly returned for columns defined with DESC ordering. Ref #25
…adata Fix clustering order not propagated from driver metadata
…ylla-skips Remove notScylla skips for clustering order tests
Add -Werror to scalacOptions so the build fails on any Scala warning. Fix all existing warnings across 58 files including: implicit definitions missing explicit types, deprecated API usage (Stream, toIterator, mapValues, JavaConverters, unicode arrows, auto-application), unchecked erasure patterns, missing language feature imports, and non-exhaustive pattern matches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-WerrortoscalacOptionsinbuild.sbtso the build fails on any Scala compiler warningdriver/,connector/, andtest-support/modulesWarning categories fixed
()to nullary method calls (MultiplexingSchemaListener, CassandraScanBuilder, openSession, createResultProjection, etc.)mapValues→.view.mapValues().toMap,Stream→LazyList,toIterator→iterator,toStream→.to(LazyList),JavaConverters→jdk.CollectionConverters.toIndexedSeqcalls.toIndexedSeqbefore: _*splats→/⇒with->/=>implicitConversions,existentials,reflectiveCallswhere needed@nowarn("cat=deprecation")where no alternative exists (JaroWinkler, getNodeFilter, Date methods)@uncheckedannotations'namewithSymbol("name")Class.newInstance()— replaced withgetDeclaredConstructor().newInstance()Test plan
./sbt/sbt clean compilesucceeds with zero warnings./sbt/sbt clean test:compilesucceeds with zero warnings./sbt/sbt test— all 320 unit tests pass🤖 Generated with Claude Code