[Data Validation] Add table configuration - #4188
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces table-level filtering for the GCS-to-Spanner Data Validation pipeline. By allowing users to specify a subset of tables via a comma-separated list or a GCS file path, the pipeline can now perform targeted validation, significantly improving efficiency when working with large schemas. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces table-based filtering to the GCS to Spanner Data Validation pipeline, allowing users to specify a subset of tables to validate via a comma-separated list or a GCS file path. Feedback on the changes focuses on improving robustness and code quality, including adding defensive null checks to prevent potential NullPointerExceptions in SourceReaderTransform, TableSelectionConfig, and CreateSpannerReadOpsFn. Additionally, it is recommended to use StandardCharsets.UTF_8 instead of string literals, and to remove a redundant Thread.sleep call in the integration tests as the Dataflow runner's startup time is already sufficient to satisfy the staleness window.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4188 +/- ##
=============================================
+ Coverage 33.88% 63.20% +29.31%
- Complexity 503 2816 +2313
=============================================
Files 215 564 +349
Lines 13133 32721 +19588
Branches 1304 3671 +2367
=============================================
+ Hits 4450 20681 +16231
- Misses 8318 10987 +2669
- Partials 365 1053 +688
🚀 New features to boost your workflow:
|
I don't think it should be flat text file, I think it should be a |
manitgupta
left a comment
There was a problem hiding this comment.
I haven't looked at the full PR yet, lets align on the existing comments and then will look at the rest
| * Configuration class for table-based filtering in Data Validation pipeline. Encapsulates parsing, | ||
| * matching, and validation of source and Spanner tables. | ||
| */ | ||
| public class TableSelectionConfig implements Serializable { |
There was a problem hiding this comment.
I think this should be a named TableConfiguration (mentioned the reason in a different comment - extensibility).
The TableConfiguration should have a field tableName which is used for filtering.
There was a problem hiding this comment.
will make these changes + JSON and get back
92afbd3 to
9323e2f
Compare
|
Can you put a sample |
b/514940857
Added two new pipeline parameters:
tables: comma-separated list of source tables for small configurationstableListFilePath: GCS File path containing list of source tables in a text file for large workloadsDesign Decisions
Why two different template params?
tableswill be extremely inconvenient to configure in case user needs to configure large number of tablestableListFilePathis JSON fileExtensible for future additions
Example:
Table names are to be specified as per their source names
More intuitive for users as they are more familiar with their source schemas. In case of Spanner table renames, the user specifies an overrides file/session file - which will be used to map their configured tables to their corresponding Spanner names
Fail fast if both template params are provided
There should be only one source of truth for the configured tables and since the purpose for both template params is the same, it doesn't make sense to pass both.
No mechanism to fail fast if configured tables do not exist in source
Since our source is the GCS Avro directory, if a source table exists but has zero rows, it will not have any entry in GCS.
Therefore, GCS records for a table not existing could mean either of two things:
Since there's no way to differentiate between these two cases, we cannot introduce a fail-fast loop to identify a misconfigured table list.
Testing
tablesmentions a subset of tables which were renamed in spanner (so tests table config in combination with overrides)