Add spannerChangeStreamTvfNameList parameters for 3 change stream templates#3838
Add spannerChangeStreamTvfNameList parameters for 3 change stream templates#3838tianz101 wants to merge 3 commits into
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 a new configuration parameter, 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates Beam dependencies to version 2.74.0 and introduces a new spannerChangeStreamTvfNameList parameter to the Spanner change stream templates for BigQuery, GCS, and Pub/Sub. This allows for querying and unioning multiple TVF names. Feedback includes correcting a recurring typo ('glound' to 'Google Cloud') in the parameter help text across several files and removing a redundant null check in the BigQuery template logic.
| String tvfNameListString = options.getSpannerChangeStreamTvfNameList(); | ||
| if (tvfNameListString != null && !tvfNameListString.isEmpty()) { | ||
| List<String> tvfNameList = Arrays.asList(tvfNameListString.split(":")); | ||
| if (tvfNameList != null && !tvfNameList.isEmpty()) { |
There was a problem hiding this comment.
The null check for tvfNameList is redundant because Arrays.asList never returns null. Mandatory parameters should be validated at the entry point to avoid redundant null checks downstream.
| if (tvfNameList != null && !tvfNameList.isEmpty()) { | |
| if (!tvfNameList.isEmpty()) { |
References
- Ensure that mandatory parameters are validated for nullability at the entry point or in upstream components to prevent NullPointerExceptions, avoiding the need for redundant null checks.
There was a problem hiding this comment.
Arrays.asList() never returns null in Java. We can use if (!tvfNameList.isEmpty()) { here directly.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3838 +/- ##
=============================================
+ Coverage 33.99% 55.50% +21.51%
- Complexity 501 6573 +6072
=============================================
Files 215 1103 +888
Lines 13074 67605 +54531
Branches 1290 7587 +6297
=============================================
+ Hits 4444 37522 +33078
- Misses 8274 27667 +19393
- Partials 356 2416 +2060
🚀 New features to boost your workflow:
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
tianz101
force-pushed
the
main
branch
4 times, most recently
from
192e8af to
ae6b6c5
Compare
| String tvfNameListString = options.getSpannerChangeStreamTvfNameList(); | ||
| if (tvfNameListString != null && !tvfNameListString.isEmpty()) { | ||
| List<String> tvfNameList = Arrays.asList(tvfNameListString.split(":")); | ||
| if (tvfNameList != null && !tvfNameList.isEmpty()) { |
There was a problem hiding this comment.
Arrays.asList() never returns null in Java. We can use if (!tvfNameList.isEmpty()) { here directly.
|
|
||
| String tvfNameListString = options.getSpannerChangeStreamTvfNameList(); | ||
| if (tvfNameListString != null && !tvfNameListString.isEmpty()) { | ||
| List<String> tvfNameList = Arrays.asList(tvfNameListString.split(";")); |
There was a problem hiding this comment.
Issue: If a user provides input like tvf1;tvf2; (with a trailing semicolon), the split() method will create an empty string as the last element:
"tvf1;tvf2;".split(";") // Results in: ["tvf1", "tvf2", ""]
Recommendation: Filter out empty strings:
List tvfNameList = Arrays.stream(tvfNameListString.split(";"))
.filter(name -> !name.trim().isEmpty())
.collect(Collectors.toList());
|
There is spotless format check failure. Need to run mvn spotless:apply to format changes. |
2 participants
We are adding spannerChangeStreamTvfNameList paramter to allow users to
specify the change stream to query or the list of tvf names to query.