Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft: Not for review] Changes to use Existing cassandra io for Bulk Cassandra Read with SSL #2114

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

VardhanThigle
Copy link
Contributor

@VardhanThigle VardhanThigle commented Jan 6, 2025

Overview

Changes to use Existing CassandraIO for Cassandra Bulk Read.

Note on Techdept

The existing CassandraIO works with Cassandra Driver's 3.0 interfaces. This would mean that we won't get out of box support for all the new features and configurations that 4.0 driver provides out of box. With this PR we are enabling the most basic use case of reading a primitive types on SSL enabled Cluster with client side validation. Mutual TLS and other advanced options are not implemented currently.
The strategy to eliminate this techdebt is to enable 4.0 in upstream code, once the Cassandra Bulk is working for basic use cases.
Till then, if we see any specific customer use case, we can manually incorporate it (the way SSL is currently handled).

PR Tree:

At the moment the PR is large. To de-risk having a lot of code for review, we have raised 2 child PRs:

  1. Basic Changes to use Upstream Cassandra io for Bulk Cassandra Migration  #2129 - Has the first 2 commits, and gets basic CassandraIO working.
  2. Enable SSL mode in embedded Cassandra used by unit tests. #2130 - UT only code to have an ssl enabled Cassandra Server for UT.
  3. [Draft: Not for review] Changes to use Existing cassandra io for Bulk Cassandra Read with SSL #2114 - The Current PR which in addition to Basic Changes to use Upstream Cassandra io for Bulk Cassandra Migration  #2129 and Enable SSL mode in embedded Cassandra used by unit tests. #2130 also enables Read from SSL enabled Cassandra Clusters for the most basic use case. This PR will become smaller once Basic Changes to use Upstream Cassandra io for Bulk Cassandra Migration  #2129 and Enable SSL mode in embedded Cassandra used by unit tests. #2130 are merged to main line.

@VardhanThigle VardhanThigle force-pushed the existing-cassandra-io branch from 7c79275 to 02378ed Compare January 6, 2025 10:07
@VardhanThigle VardhanThigle force-pushed the existing-cassandra-io branch 4 times, most recently from 19833f9 to bea08e7 Compare January 6, 2025 11:17
@VardhanThigle VardhanThigle force-pushed the existing-cassandra-io branch from bea08e7 to 33a2032 Compare January 6, 2025 11:22
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 54.97%. Comparing base (7147be4) to head (c5a53e4).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...er/CassandraTableReaderFactoryCassandraIoImpl.java 94.28% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2114      +/-   ##
============================================
+ Coverage     46.61%   54.97%   +8.35%     
+ Complexity     4278     1608    -2670     
============================================
  Files           868      406     -462     
  Lines         51581    21864   -29717     
  Branches       5401     2161    -3240     
============================================
- Hits          24046    12019   -12027     
+ Misses        25815     9147   -16668     
+ Partials       1720      698    -1022     
Components Coverage Δ
spanner-templates 70.43% <98.60%> (+1.83%) ⬆️
spanner-import-export ∅ <ø> (∅)
spanner-live-forward-migration 77.51% <ø> (+0.02%) ⬆️
spanner-live-reverse-replication 78.45% <ø> (+0.02%) ⬆️
spanner-bulk-migration 87.91% <98.60%> (+0.24%) ⬆️
Files with missing lines Coverage Δ
...der/io/cassandra/iowrapper/SSLOptionsProvider.java 100.00% <100.00%> (ø)
...er/CassandraTableReaderFactoryCassandraIoImpl.java 97.05% <94.28%> (ø)

... and 485 files with indirect coverage changes

@VardhanThigle VardhanThigle marked this pull request as ready for review January 6, 2025 11:37
@VardhanThigle VardhanThigle requested a review from a team as a code owner January 6, 2025 11:37
@VardhanThigle VardhanThigle force-pushed the existing-cassandra-io branch 3 times, most recently from ad50525 to 4139891 Compare January 13, 2025 04:07
@VardhanThigle VardhanThigle changed the title Changes to use Existing cassandra io for Bulk Cassandra Read [Draft: Not for review] Changes to use Existing cassandra io for Bulk Cassandra Read Jan 13, 2025
@VardhanThigle VardhanThigle marked this pull request as draft January 13, 2025 05:43
@VardhanThigle VardhanThigle changed the title [Draft: Not for review] Changes to use Existing cassandra io for Bulk Cassandra Read [Draft: Not for review] Changes to use Existing cassandra io for Bulk Cassandra Read with SSL Jan 13, 2025
@VardhanThigle VardhanThigle force-pushed the existing-cassandra-io branch 3 times, most recently from 33c1ba0 to c5a53e4 Compare January 14, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant