Revert #37631 and #38497 on HEAD#38516
Conversation
…)" This reverts commit 81769cb.
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 reverts two previous changes (#37631 and #38497) that introduced regressions in non-Java runners and caused issues with internal imports. The changes involve removing the recently added SchemaCoder support from the model coder registry, simplifying the SdkComponents interface by removing its dependency on PipelineOptions, and cleaning up the CoderTranslatorRegistrar interface. These changes ensure the stability of the release branch. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 refactors SdkComponents to remove its dependency on PipelineOptions, simplifies coder translation by removing SchemaCoder, and updates DataflowRunner terminology to v1/v2. Review feedback points out typos in log messages and comments, a potential NullPointerException in CoderTranslation due to a removed null check, and a missing newline in CHANGES.md.
| } else { | ||
| LOG.warn( | ||
| "Using use_deprecated_read in portable runners is runner-dependent. The " | ||
| "Using use_depreacted_read in portable runners is runner-dependent. The " |
| Environments.createDockerEnvironment(workerHarnessContainerImageURL); | ||
|
|
||
| // The SdkComponents for portable and non-portable job submission must be kept distinct. Both | ||
| // The SdkComponents for portable an non-portable job submission must be kept distinct. Both |
| // Now rewrite things to be as needed for v1 (mutates the pipeline) | ||
| // This way the job submitted is valid for v1 and v2, simultaneously |
There was a problem hiding this comment.
Comments should end with a period for better readability and consistency with the rest of the codebase.
| // Now rewrite things to be as needed for v1 (mutates the pipeline) | |
| // This way the job submitted is valid for v1 and v2, simultaneously | |
| // Now rewrite things to be as needed for v1 (mutates the pipeline). | |
| // This way the job submitted is valid for v1 and v2, simultaneously. |
| /** | ||
| * Create new {@link SdkComponents} importing all items from provided {@link Components} object. | ||
| * | ||
| * <p>WARNING: This action might cause some of duplicate items created. |
There was a problem hiding this comment.
| if (translator == null) { | ||
| throw new IOException("Unable to find CoderTranslator for known Coder"); | ||
| } | ||
| CoderTranslator translator = getKnownTranslators().get(coder.getClass()); |
There was a problem hiding this comment.
The explicit null check and descriptive IOException were removed. If a translator is missing for a known coder, this will now result in a NullPointerException during the call to registerComponents. It is safer to keep the check and throw a descriptive exception to aid debugging.
CoderTranslator translator = getKnownTranslators().get(coder.getClass());
if (translator == null) {
throw new IOException("Unable to find CoderTranslator for known Coder: " + coder.getClass());
}| ## Highlights | ||
|
|
||
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | ||
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). No newline at end of file |
There was a problem hiding this comment.
The file is missing a newline at the end. It is a best practice to end text files with a newline character to ensure compatibility with various Unix tools and to avoid 'No newline at end of file' warnings in diffs.
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | |
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
rrio test failure not related to this change |
Please add a meaningful description for your change here
Revert #37631 and #38497 in the release branch
Reason: break validate runner tests in non-Java runners; break internal import
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.