Skip to content

Commit 32dbe8d

Browse files
committed
Fix addExperiment() to handle immutable lists and remove redundant defensive copies
1 parent 99ddb84 commit 32dbe8d

4 files changed

Lines changed: 2 additions & 15 deletions

File tree

runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,6 @@ public Translator(Pipeline pipeline, DataflowRunner runner, SdkComponents sdkCom
403403
* @return a Job definition filled in with the type of job, the environment, and the job steps.
404404
*/
405405
public Job translate(List<DataflowPackage> packages) {
406-
// Ensure the experiments list is mutable before any experiments are added.
407-
if (options.getExperiments() != null) {
408-
options.setExperiments(new ArrayList<>(options.getExperiments()));
409-
}
410406
job.setName(options.getJobName().toLowerCase());
411407

412408
Environment environment = new Environment();

runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,10 +1243,6 @@ private static boolean includesTransformUpgrades(Pipeline pipeline) {
12431243
@SuppressWarnings("Slf4jFormatShouldBeConst")
12441244
@Override
12451245
public DataflowPipelineJob run(Pipeline pipeline) {
1246-
// Ensure the experiments list is mutable before any experiments are added.
1247-
if (options.getExperiments() != null) {
1248-
options.setExperiments(new ArrayList<>(options.getExperiments()));
1249-
}
12501246
// Multi-language pipelines and pipelines that include upgrades should automatically be upgraded
12511247
// to Dataflow Portable Runner.
12521248
if (DataflowRunner.isMultiLanguagePipeline(pipeline) || includesTransformUpgrades(pipeline)) {

runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/client/grpc/GrpcWindmillServer.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,6 @@ private static DataflowWorkerHarnessOptions testOptions(
111111
boolean enableStreamingEngine, List<String> additionalExperiments) {
112112
DataflowWorkerHarnessOptions options =
113113
PipelineOptionsFactory.create().as(DataflowWorkerHarnessOptions.class);
114-
// Ensure the experiments list is mutable before any experiments are added.
115-
if (options.getExperiments() != null) {
116-
options.setExperiments(new ArrayList<>(options.getExperiments()));
117-
}
118114
options.setProject("project");
119115
options.setJobId("job");
120116
options.setWorkerId("worker");

sdks/java/core/src/main/java/org/apache/beam/sdk/options/ExperimentalOptions.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.apache.beam.sdk.options;
1919

20+
import java.util.ArrayList;
2021
import java.util.List;
2122
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.Lists;
2223
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -57,9 +58,7 @@ static boolean hasExperiment(PipelineOptions options, String experiment) {
5758
/** Adds experiment to options if not already present. */
5859
static void addExperiment(ExperimentalOptions options, String experiment) {
5960
List<String> experiments = options.getExperiments();
60-
if (experiments == null) {
61-
experiments = Lists.newArrayList();
62-
}
61+
experiments = (experiments == null) ? Lists.newArrayList() : new ArrayList<>(experiments);
6362
if (!experiments.contains(experiment)) {
6463
experiments.add(experiment);
6564
}

0 commit comments

Comments
 (0)