gemini-code-assist[bot] commented on code in PR #38516:
URL: https://github.com/apache/beam/pull/38516#discussion_r3249349953
##########
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java:
##########
@@ -600,7 +600,7 @@ private Map<String, TransformProvider>
loadRegisteredTransforms() {
pipeline.getOptions().as(ExperimentalOptions.class), "use_sdf_read");
} 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 "
Review Comment:
There is a typo in the log message: `use_depreacted_read` should be
`use_deprecated_read`.
```suggestion
"Using use_deprecated_read in portable runners is
runner-dependent. The "
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java:
##########
@@ -1333,20 +1333,19 @@ public DataflowPipelineJob run(Pipeline pipeline) {
// with the SDK harness image (which implements Fn API).
//
// The same Environment is used in different and contradictory ways,
depending on whether
- // it is a portable or non-portable job submission.
+ // it is a v1 or v2 job submission.
RunnerApi.Environment defaultEnvironmentForDataflow =
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
Review Comment:
There is a typo in the comment: `an` should be `and`.
```suggestion
// The SdkComponents for portable and non-portable job submission must
be kept distinct. Both
```
##########
CHANGES.md:
##########
@@ -2441,4 +2438,4 @@ Schema Options, it will be removed in version `2.23.0`.
([BEAM-9704](https://iss
## 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/).
Review 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.
```suggestion
- For versions 2.19.0 and older release notes are available on [Apache Beam
Blog](https://beam.apache.org/blog/).
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java:
##########
@@ -1375,30 +1374,28 @@ public DataflowPipelineJob run(Pipeline pipeline) {
options.as(SdkHarnessOptions.class).setPipelineProtoHash(pipelineProtoHash);
if (useUnifiedWorker(options)) {
- LOG.info(
- "Skipping non-portable transform replacements since job will run on
portable worker.");
+ LOG.info("Skipping v1 transform replacements since job will run on v2.");
} else {
- // Now rewrite things to be as needed for non-portable (mutates the
pipeline).
- // This way the job submitted is valid for portable and non-portable,
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
Review Comment:
Comments should end with a period for better readability and consistency
with the rest of the codebase.
```suggestion
// Now rewrite things to be as needed for v1 (mutates the pipeline).
// This way the job submitted is valid for v1 and v2, simultaneously.
```
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/CoderTranslation.java:
##########
@@ -180,10 +129,7 @@ private static RunnerApi.Coder
toUnknownCoderWrapper(UnknownCoderWrapper coder)
private static RunnerApi.Coder toKnownCoder(Coder<?> coder, SdkComponents
components)
throws IOException {
- CoderTranslator translator = getCoderTranslator(coder.getClass());
- if (translator == null) {
- throw new IOException("Unable to find CoderTranslator for known Coder");
- }
+ CoderTranslator translator = getKnownTranslators().get(coder.getClass());
Review 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.
```java
CoderTranslator translator = getKnownTranslators().get(coder.getClass());
if (translator == null) {
throw new IOException("Unable to find CoderTranslator for known Coder:
" + coder.getClass());
}
```
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/SdkComponents.java:
##########
@@ -72,7 +71,17 @@ public class SdkComponents {
/** Create a new {@link SdkComponents} with no components. */
public static SdkComponents create() {
- return new SdkComponents(RunnerApi.Components.getDefaultInstance(), null,
"", null);
+ return new SdkComponents(RunnerApi.Components.getDefaultInstance(), null,
"");
+ }
+
+ /**
+ * Create new {@link SdkComponents} importing all items from provided {@link
Components} object.
+ *
+ * <p>WARNING: This action might cause some of duplicate items created.
Review Comment:
The Javadoc contains a grammatical error: 'some of duplicate items created'.
It should be 'some duplicate items to be created'.
```suggestion
* <p>WARNING: This action might cause some duplicate items to be created.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
