gemini-code-assist[bot] commented on code in PR #38252:
URL: https://github.com/apache/beam/pull/38252#discussion_r3407402446
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java:
##########
@@ -1243,16 +1243,21 @@ private static boolean
includesTransformUpgrades(Pipeline pipeline) {
@SuppressWarnings("Slf4jFormatShouldBeConst")
@Override
public DataflowPipelineJob run(Pipeline pipeline) {
+ // Ensure the experiments list is mutable before any experiments are added.
+ if (options.getExperiments() != null) {
+ options.setExperiments(new ArrayList<>(options.getExperiments()));
+ }
// Multi-language pipelines and pipelines that include upgrades should
automatically be upgraded
Review Comment:
This pre-emptive copy of the experiments list to an ArrayList is redundant.
ExperimentalOptions.addExperiment already handles null-initialization and
copies the existing list into a new mutable ArrayList internally before adding
any new experiments. You can safely remove this block.
```suggestion
// Multi-language pipelines and pipelines that include upgrades should
automatically be upgraded
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java:
##########
@@ -402,6 +403,10 @@ public Translator(Pipeline pipeline, DataflowRunner
runner, SdkComponents sdkCom
* @return a Job definition filled in with the type of job, the
environment, and the job steps.
*/
public Job translate(List<DataflowPackage> packages) {
+ // Ensure the experiments list is mutable before any experiments are
added.
+ if (options.getExperiments() != null) {
+ options.setExperiments(new ArrayList<>(options.getExperiments()));
+ }
job.setName(options.getJobName().toLowerCase());
Review Comment:
This pre-emptive copy of the experiments list to an ArrayList is redundant.
ExperimentalOptions.addExperiment already handles null-initialization and
copies the existing list into a new mutable ArrayList internally before adding
any new experiments. You can safely remove this block.
```suggestion
job.setName(options.getJobName().toLowerCase());
```
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/client/grpc/GrpcWindmillServer.java:
##########
@@ -110,16 +111,20 @@ private static DataflowWorkerHarnessOptions testOptions(
boolean enableStreamingEngine, List<String> additionalExperiments) {
DataflowWorkerHarnessOptions options =
PipelineOptionsFactory.create().as(DataflowWorkerHarnessOptions.class);
+ // Ensure the experiments list is mutable before any experiments are added.
+ if (options.getExperiments() != null) {
+ options.setExperiments(new ArrayList<>(options.getExperiments()));
+ }
options.setProject("project");
Review Comment:
This pre-emptive copy of the experiments list to an ArrayList is redundant.
ExperimentalOptions.addExperiment already handles null-initialization and
copies the existing list into a new mutable ArrayList internally before adding
any new experiments. You can safely remove this block.
```suggestion
options.setProject("project");
```
--
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]
