feat(Spanner): integrate SourceConfigParser to centralize shard configuration loading for SpannerToSourceDb pipelines by pratickchokhani · Pull Request #3840 · GoogleCloudPlatform/DataflowTemplates

pratickchokhani · 2026-05-21T07:05:34Z

No description provided.

codecov · 2026-05-21T07:20:54Z

Codecov Report

❌ Patch coverage is 66.66667% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.47%. Comparing base (d8c6ecc) to head (9014e52).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...cloud/teleport/v2/templates/SpannerToSourceDb.java	62.06%	8 Missing and 3 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #3840       +/-   ##
=============================================
+ Coverage     39.94%   55.47%   +15.52%     
- Complexity      684     6509     +5825     
=============================================
  Files           208     1102      +894     
  Lines         14902    67463    +52561     
  Branches       1528     7567     +6039     
=============================================
+ Hits           5953    37425    +31472     
- Misses         8451    27591    +19140     
- Partials        498     2447     +1949

Components	Coverage Δ
spanner-templates	`87.80% <66.66%> (∅)`
spanner-import-export	`68.61% <ø> (∅)`
spanner-live-forward-migration	`90.22% <100.00%> (∅)`
spanner-live-reverse-replication	`82.87% <66.66%> (∅)`
spanner-bulk-migration	`92.62% <100.00%> (∅)`
gcs-spanner-dv	`89.08% <100.00%> (∅)`

Files with missing lines	Coverage Δ
...er/migrations/utils/CassandraConfigFileReader.java	`100.00% <100.00%> (ø)`
...cloud/teleport/v2/templates/SpannerToSourceDb.java	`19.51% <62.06%> (ø)`

... and 917 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pratickchokhani · 2026-05-21T09:38:56Z

    ResultSet mockRs = mock(ResultSet.class);

    when(mockConn.createStatement()).thenReturn(mockStmt);
-    when(mockStmt.executeQuery("SHOW VARIABLES LIKE 'read_only'")).thenReturn(mockRs);


It's removed as mokito logs show that these mocks are not used in the test.

gemini-code-assist · 2026-05-26T08:59:46Z

Warning

Gemini encountered an error creating the summary. You can try again by commenting /gemini summary.

bharadwaj-aditya · 2026-05-27T06:08:45Z

+    // cassandra is always a single sharded migration.
+    // for JDBC, shards size and IsShardedMigration option is used below.
+    String shardingMode =
+        options.getSourceType().equals(CASSANDRA_SOURCE_TYPE)


This should be moved to the respective config parsing flow rather than having it as a top level decision

How does the non-sharded workflow work here ?

shardingMode: this is used in AssignShardIdFn to identify if default shard id is needed to be assigned to stream events.

I could have used the size of shards list to identify shardingMode.

The primary concern in doing so was the logic below (at line 672) where this is also dependent on the user input getIsShardedMigration.

We can AND that flag here to get to the output. In any case, not a blocker to letting this through. Lets take it up as a follow up

bharadwaj-aditya · 2026-05-27T06:10:35Z

+    List<Shard> shards;
+    if (sourceConnectionConfig instanceof JdbcShardConfig) {
+      shards = ((JdbcShardConfig) sourceConnectionConfig).getShardConfigs();
+      LOG.info("JDBC config is: {}", shards);


do not log this, this might have PII

bharadwaj-aditya · 2026-05-27T06:10:50Z

+      shards =
+          cassandraConfigFileReader.getCassandraShard(
+              ((CassandraConnectionConfig) sourceConnectionConfig).getOptionsMap());
+      LOG.info("Cassandra config is: {}", shards.get(0));


Do not log. It might have PII

bharadwaj-aditya · 2026-05-27T06:13:14Z

                    <groupId>com.datastax.cassandra</groupId>
                    <artifactId>cassandra-driver-core</artifactId>
                </exclusion>
+                <exclusion>


why is this excluded ? Is it a change in beam ?

Without this exclusion, there is a version conflict with:
org.apache.cassandra
java-driver-core
${cassandra-java-driver-core.version}

This dependency is test scoped. It was not causing any issue earlier as we didn't have tests which was using this package.

bharadwaj-aditya · 2026-05-27T06:14:46Z

 9. Configuration Files Upload
    - **For MySQL:**
-      [Source shards file](./RunnigReverseReplication.md#sample-source-shards-file-for-MySQL) already uploaded to GCS.
+      [source shards file](#sample-source-shards-file-for-MySQL) already uploaded to GCS.


Please keep the capitalization here for uniformity

bharadwaj-aditya · 2026-05-27T06:15:54Z

+   * @param sourceShardsFilePath The GCS path to the source shards configuration file.
+   * @return A list of shards.
+   */
+  public static List<Shard> getShardList(String sourceType, String sourceShardsFilePath) {


ideally this method should move to spanner-common

The logic below is specific to Reverse replication. There is no synergy on this between reverse and bulk.

Also, the plan is to remove this in phase 2. I have added a comment to reflect this.

…guration loading for SpannerToSourceDb pipelines

bharadwaj-aditya

There is some follow up work here. But marking as LGTM as those items can be taken up in subsequent PRS

bharadwaj-aditya · 2026-05-27T14:04:16Z

+    // cassandra is always a single sharded migration.
+    // for JDBC, shards size and IsShardedMigration option is used below.
+    String shardingMode =
+        options.getSourceType().equals(CASSANDRA_SOURCE_TYPE)


We can AND that flag here to get to the output. In any case, not a blocker to letting this through. Lets take it up as a follow up

bharadwaj-aditya

LGTM

pull-request-size Bot added the size/XL label May 21, 2026

pratickchokhani added the addition New feature or request label May 21, 2026

pratickchokhani force-pushed the shard-config-reverse branch from aeaefa2 to ca0311c Compare May 21, 2026 07:08

pratickchokhani force-pushed the shard-config-reverse branch from ca0311c to 87ab9eb Compare May 21, 2026 09:20

pull-request-size Bot added size/XXL and removed size/XL labels May 21, 2026

pratickchokhani force-pushed the shard-config-reverse branch 3 times, most recently from 7a2cdc1 to 6436182 Compare May 21, 2026 09:37

pratickchokhani commented May 21, 2026

View reviewed changes

pratickchokhani force-pushed the shard-config-reverse branch 19 times, most recently from 08bd358 to 213e4aa Compare May 26, 2026 08:41

pratickchokhani requested a review from bharadwaj-aditya May 26, 2026 08:41

pratickchokhani marked this pull request as ready for review May 26, 2026 08:41

pratickchokhani requested a review from a team as a code owner May 26, 2026 08:41

pratickchokhani requested a review from shreyakhajanchi May 26, 2026 08:41

bharadwaj-aditya reviewed May 27, 2026

View reviewed changes

pratickchokhani force-pushed the shard-config-reverse branch 3 times, most recently from f52391a to f6ce311 Compare May 27, 2026 10:13

feat(Spanner): integrate SourceConfigParser to centralize shard confi…

9014e52

…guration loading for SpannerToSourceDb pipelines

pratickchokhani force-pushed the shard-config-reverse branch from f6ce311 to 9014e52 Compare May 27, 2026 10:14

pratickchokhani requested a review from bharadwaj-aditya May 27, 2026 10:35

bharadwaj-aditya reviewed May 27, 2026

View reviewed changes

bharadwaj-aditya approved these changes May 27, 2026

View reviewed changes

Conversation

pratickchokhani commented May 21, 2026

Uh oh!

codecov Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot commented May 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bharadwaj-aditya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bharadwaj-aditya left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented May 21, 2026 •

edited

Loading