Add test to cover SDF with side input data correctness#35951
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @apanich, 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 test case to the Apache Beam Python SDK, specifically targeting the data consistency of side inputs when used in conjunction with Splittable DoFns (SDFs). The added test validates that side input data remains accurate and complete throughout the processing pipeline, even when the main input is handled by a splittable DoFn. This enhancement improves the robustness and reliability of Beam pipelines that utilize these advanced features.
Highlights
- New Test for SDF Side Input Consistency: A new test,
test_side_input_with_sdf, has been added tosdks/python/apache_beam/transforms/sideinputs_test.py. This test is crucial for ensuring the reliability of side inputs when processed by Splittable DoFns (SDFs). - Data Consistency Verification Method: The test verifies data consistency by comparing the size and a cryptographic fingerprint of the side input, ensuring that the data remains intact and correct even when split and processed by SDFs. It uses
SyntheticSDFAsSourceto simulate the SDF behavior. - New Helper Function for Data Fingerprinting: A utility function,
fingerprint_list, was introduced to generate a stable MD5 hash of a list of elements, which is used to validate the integrity of the side input data. - Specific Test Environment Requirements: The test includes specific runner requirements (e.g., at least 2 workers, autoscaling disabled) to ensure that the conditions for exposing potential consistency issues are met, preventing false positives.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
R: @Abacn |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a new integration test to verify the data consistency of side inputs when used with Splittable DoFns (SDF). The test uses a synthetic SDF source to generate a large side input and then checks its size and fingerprint. The changes are well-structured. I've found a small amount of dead code that should be removed and a minor opportunity for optimization in the test logic.
| def fingerprint_list(elements: List[Any]) -> str: | ||
| """Computes a stable fingerprint for an iterable of elements.""" | ||
| # Sort and convert to string for consistent hashing. | ||
| s = str(sorted(elements)) | ||
| return hashlib.md5(s.encode()).hexdigest() | ||
|
|
There was a problem hiding this comment.
This fingerprint_list function is not used anywhere in the file. It seems like it was intended for use, but a different implementation was chosen for the fingerprint calculation inside SideInputTrackingDoFn. To keep the code clean, this unused function should be removed.
| side_input_list = list(side_input) | ||
| size = len(side_input_list) | ||
| # Sort for consistent hashing. | ||
| m = hashlib.md5() | ||
| for key, value in sorted(side_input_list): |
There was a problem hiding this comment.
The current implementation materializes the side_input iterable into a list and then sorts it, which involves creating two lists. This can be slightly optimized by calling sorted() directly on the iterable, which creates only one list and can be more readable.
| side_input_list = list(side_input) | |
| size = len(side_input_list) | |
| # Sort for consistent hashing. | |
| m = hashlib.md5() | |
| for key, value in sorted(side_input_list): | |
| sorted_side_input = sorted(side_input) | |
| size = len(sorted_side_input) | |
| # Sort for consistent hashing. | |
| m = hashlib.md5() | |
| for key, value in sorted_side_input: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #35951 +/- ##
============================================
+ Coverage 54.76% 54.82% +0.06%
Complexity 1646 1646
============================================
Files 1060 1061 +1
Lines 163562 163923 +361
Branches 1189 1189
============================================
+ Hits 89567 89866 +299
- Misses 71843 71905 +62
Partials 2152 2152
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Would you mind creating a file |
apanich
force-pushed
the
add-sdf-sideinput-test
branch
from
7cff8d0 to
955cc65
Compare
done |
|
1 out of 2 runs failed: test_ml_preprocessing_yaml (apache_beam.yaml.examples.testing.examples_test.MLTest) not related to the change, merging for now |
3 participants
Adds beam test for side input with SDF (splittable DoFn) to verify data consistency
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.