Skip to content

Commit 2e97f56

Browse files
authored
[GOBBLIN-2078] remove configs because they may have a config that is prefix of another config (apache#3961)
1 parent 290d17d commit 2e97f56

File tree

9 files changed

+35
-50
lines changed

9 files changed

+35
-50
lines changed

gobblin-modules/gobblin-troubleshooter/src/test/java/org/apache/gobblin/troubleshooter/AutomaticTroubleshooterTest.java

+3-8
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.apache.gobblin.metrics.event.GobblinEventBuilder;
2929
import org.apache.gobblin.runtime.troubleshooter.AutomaticTroubleshooter;
3030
import org.apache.gobblin.runtime.troubleshooter.AutomaticTroubleshooterFactory;
31-
import org.apache.gobblin.util.ConfigUtils;
3231

3332
import static org.mockito.Mockito.*;
3433
import static org.testng.Assert.assertEquals;
@@ -41,9 +40,7 @@ public class AutomaticTroubleshooterTest {
4140
@Test
4241
public void canCollectAndRefineIssues()
4342
throws Exception {
44-
Properties properties = new Properties();
45-
AutomaticTroubleshooter troubleshooter =
46-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(properties));
43+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(new Properties());
4744
try {
4845
troubleshooter.start();
4946
log.warn("Test warning");
@@ -72,8 +69,7 @@ public void canDisable()
7269
throws Exception {
7370
Properties properties = new Properties();
7471
properties.put(ConfigurationKeys.TROUBLESHOOTER_DISABLED, "true");
75-
AutomaticTroubleshooter troubleshooter =
76-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(properties));
72+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(properties);
7773
try {
7874
troubleshooter.start();
7975
log.warn("Test warning");
@@ -96,8 +92,7 @@ public void canDisableEventReporter()
9692
Properties properties = new Properties();
9793
properties.put(ConfigurationKeys.TROUBLESHOOTER_DISABLED, "false");
9894
properties.put(ConfigurationKeys.TROUBLESHOOTER_DISABLE_EVENT_REPORTING, "true");
99-
AutomaticTroubleshooter troubleshooter =
100-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(properties));
95+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(properties);
10196
try {
10297
troubleshooter.start();
10398

gobblin-runtime/src/main/java/org/apache/gobblin/runtime/AbstractJobLauncher.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@
3131
import java.util.concurrent.ExecutionException;
3232
import java.util.concurrent.ExecutorService;
3333
import java.util.concurrent.Executors;
34-
3534
import java.util.concurrent.atomic.AtomicInteger;
36-
import lombok.Getter;
35+
3736
import org.apache.commons.lang.StringUtils;
3837
import org.slf4j.Logger;
3938
import org.slf4j.LoggerFactory;
@@ -57,6 +56,7 @@
5756
import com.typesafe.config.ConfigFactory;
5857

5958
import javax.annotation.Nullable;
59+
import lombok.Getter;
6060
import lombok.RequiredArgsConstructor;
6161

6262
import org.apache.gobblin.broker.SharedResourcesBrokerFactory;
@@ -205,7 +205,7 @@ public AbstractJobLauncher(Properties jobProps, List<? extends Tag<?>> metadataT
205205
clusterNameTags.addAll(Tag.fromMap(ClusterNameTags.getClusterNameTags()));
206206
GobblinMetrics.addCustomTagsToProperties(jobProps, clusterNameTags);
207207

208-
troubleshooter = AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(jobProps));
208+
troubleshooter = AutomaticTroubleshooterFactory.createForJob(jobProps);
209209
troubleshooter.start();
210210

211211
// Make a copy for both the system and job configuration properties and resolve the job-template if any.

gobblin-runtime/src/main/java/org/apache/gobblin/runtime/mapreduce/MRJobLauncher.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -779,8 +779,7 @@ protected void setup(Context context) {
779779
final State gobblinJobState = HadoopUtils.getStateFromConf(context.getConfiguration());
780780
TaskAttemptID taskAttemptID = context.getTaskAttemptID();
781781

782-
troubleshooter =
783-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(gobblinJobState.getProperties()));
782+
troubleshooter = AutomaticTroubleshooterFactory.createForJob(gobblinJobState.getProperties());
784783
troubleshooter.start();
785784

786785
try (Closer closer = Closer.create()) {

gobblin-runtime/src/main/java/org/apache/gobblin/runtime/troubleshooter/AutomaticTroubleshooterConfig.java

+9-10
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,15 @@
1818
package org.apache.gobblin.runtime.troubleshooter;
1919

2020
import java.util.Objects;
21-
22-
import com.typesafe.config.Config;
21+
import java.util.Properties;
2322

2423
import javax.inject.Inject;
2524
import lombok.AllArgsConstructor;
2625
import lombok.Builder;
2726
import lombok.Getter;
2827

2928
import org.apache.gobblin.configuration.ConfigurationKeys;
30-
import org.apache.gobblin.util.ConfigUtils;
29+
import org.apache.gobblin.util.PropertiesUtils;
3130

3231

3332
/**
@@ -44,15 +43,15 @@ public class AutomaticTroubleshooterConfig {
4443
private int inMemoryRepositoryMaxSize = ConfigurationKeys.DEFAULT_TROUBLESHOOTER_IN_MEMORY_ISSUE_REPOSITORY_MAX_SIZE;
4544

4645
@Inject
47-
public AutomaticTroubleshooterConfig(Config config) {
48-
Objects.requireNonNull(config, "Config cannot be null");
46+
public AutomaticTroubleshooterConfig(Properties properties) {
47+
Objects.requireNonNull(properties, "Properties cannot be null");
4948

50-
disabled = ConfigUtils.getBoolean(config, ConfigurationKeys.TROUBLESHOOTER_DISABLED, false);
51-
disableEventReporting =
52-
ConfigUtils.getBoolean(config, ConfigurationKeys.TROUBLESHOOTER_DISABLE_EVENT_REPORTING, false);
49+
disabled = PropertiesUtils.getPropAsBoolean(properties, ConfigurationKeys.TROUBLESHOOTER_DISABLED, "false");
50+
disableEventReporting = PropertiesUtils.getPropAsBoolean(properties,
51+
ConfigurationKeys.TROUBLESHOOTER_DISABLE_EVENT_REPORTING, "false");
5352

54-
inMemoryRepositoryMaxSize = ConfigUtils
55-
.getInt(config, ConfigurationKeys.TROUBLESHOOTER_IN_MEMORY_ISSUE_REPOSITORY_MAX_SIZE,
53+
inMemoryRepositoryMaxSize = PropertiesUtils.getPropAsInt(properties,
54+
ConfigurationKeys.TROUBLESHOOTER_IN_MEMORY_ISSUE_REPOSITORY_MAX_SIZE,
5655
ConfigurationKeys.DEFAULT_TROUBLESHOOTER_IN_MEMORY_ISSUE_REPOSITORY_MAX_SIZE);
5756
}
5857
}

gobblin-runtime/src/main/java/org/apache/gobblin/runtime/troubleshooter/AutomaticTroubleshooterFactory.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
package org.apache.gobblin.runtime.troubleshooter;
1919

20-
import com.typesafe.config.Config;
20+
import java.util.Properties;
2121

2222
import lombok.extern.slf4j.Slf4j;
2323

@@ -37,10 +37,10 @@ public class AutomaticTroubleshooterFactory {
3737
* If this module is missing, troubleshooter will default to a no-op implementation.
3838
*
3939
* In addition, even when the "gobblin-troubleshooter" module is present, troubleshooter can still be disabled
40-
* with {@link ConfigurationKeys.TROUBLESHOOTER_DISABLED} setting.
40+
* with {@link ConfigurationKeys#TROUBLESHOOTER_DISABLED} setting.
4141
* */
42-
public static AutomaticTroubleshooter createForJob(Config config) {
43-
AutomaticTroubleshooterConfig troubleshooterConfig = new AutomaticTroubleshooterConfig(config);
42+
public static AutomaticTroubleshooter createForJob(Properties properties) {
43+
AutomaticTroubleshooterConfig troubleshooterConfig = new AutomaticTroubleshooterConfig(properties);
4444

4545
Class troubleshooterClass = tryGetTroubleshooterClass();
4646

gobblin-runtime/src/test/java/org/apache/gobblin/runtime/troubleshooter/AutomaticTroubleshooterFactoryTest.java

+2-6
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.testng.annotations.Test;
2323

2424
import org.apache.gobblin.configuration.ConfigurationKeys;
25-
import org.apache.gobblin.util.ConfigUtils;
2625

2726
import static org.junit.Assert.assertTrue;
2827

@@ -33,9 +32,7 @@ public class AutomaticTroubleshooterFactoryTest {
3332
public void willGetNoopTroubleshooterByDefault() {
3433
// This test project does not reference gobblin-troubleshooter module, so we should get a noop-instance
3534
// of troubleshooter. See the main AutomaticTroubleshooterFactory class for details.
36-
Properties properties = new Properties();
37-
AutomaticTroubleshooter troubleshooter =
38-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(properties));
35+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(new Properties());
3936

4037
assertTrue(troubleshooter instanceof NoopAutomaticTroubleshooter);
4138
}
@@ -44,8 +41,7 @@ public void willGetNoopTroubleshooterByDefault() {
4441
public void willGetNoopTroubleshooterWhenDisabled() {
4542
Properties properties = new Properties();
4643
properties.put(ConfigurationKeys.TROUBLESHOOTER_DISABLED, "true");
47-
AutomaticTroubleshooter troubleshooter =
48-
AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(properties));
44+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(properties);
4945

5046
assertTrue(troubleshooter instanceof NoopAutomaticTroubleshooter);
5147
}

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/CommitActivityImpl.java

+7-8
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,18 @@
2929
import java.util.concurrent.ExecutionException;
3030
import java.util.stream.Collectors;
3131

32-
import javax.annotation.Nullable;
33-
import lombok.extern.slf4j.Slf4j;
32+
import org.apache.hadoop.fs.FileSystem;
33+
import org.apache.hadoop.fs.Path;
3434

3535
import com.google.api.client.util.Lists;
3636
import com.google.common.base.Function;
3737
import com.google.common.base.Strings;
3838
import com.google.common.collect.ImmutableList;
3939
import com.google.common.collect.Iterables;
40-
import io.temporal.failure.ApplicationFailure;
4140

42-
import org.apache.hadoop.fs.FileSystem;
43-
import org.apache.hadoop.fs.Path;
41+
import io.temporal.failure.ApplicationFailure;
42+
import javax.annotation.Nullable;
43+
import lombok.extern.slf4j.Slf4j;
4444

4545
import org.apache.gobblin.broker.gobblin_scopes.GobblinScopeTypes;
4646
import org.apache.gobblin.broker.iface.SharedResourcesBroker;
@@ -54,16 +54,15 @@
5454
import org.apache.gobblin.runtime.SafeDatasetCommit;
5555
import org.apache.gobblin.runtime.TaskState;
5656
import org.apache.gobblin.runtime.TaskStateCollectorService;
57-
import org.apache.gobblin.source.extractor.JobCommitPolicy;
5857
import org.apache.gobblin.runtime.troubleshooter.AutomaticTroubleshooter;
5958
import org.apache.gobblin.runtime.troubleshooter.AutomaticTroubleshooterFactory;
59+
import org.apache.gobblin.source.extractor.JobCommitPolicy;
6060
import org.apache.gobblin.temporal.ddm.activity.CommitActivity;
6161
import org.apache.gobblin.temporal.ddm.util.JobStateUtils;
6262
import org.apache.gobblin.temporal.ddm.work.CommitStats;
6363
import org.apache.gobblin.temporal.ddm.work.DatasetStats;
6464
import org.apache.gobblin.temporal.ddm.work.WUProcessingSpec;
6565
import org.apache.gobblin.temporal.ddm.work.assistance.Help;
66-
import org.apache.gobblin.util.ConfigUtils;
6766
import org.apache.gobblin.util.Either;
6867
import org.apache.gobblin.util.ExecutorsUtils;
6968
import org.apache.gobblin.util.PropertiesUtils;
@@ -87,7 +86,7 @@ public CommitStats commit(WUProcessingSpec workSpec) {
8786
JobState jobState = Help.loadJobState(workSpec, fs);
8887
optJobName = Optional.ofNullable(jobState.getJobName());
8988
SharedResourcesBroker<GobblinScopeTypes> instanceBroker = JobStateUtils.getSharedResourcesBroker(jobState);
90-
troubleshooter = AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(jobState.getProperties()));
89+
troubleshooter = AutomaticTroubleshooterFactory.createForJob(jobState.getProperties());
9190
troubleshooter.start();
9291
List<TaskState> taskStates = loadTaskStates(workSpec, fs, jobState, numDeserializationThreads);
9392
if (taskStates.isEmpty()) {

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/GenerateWorkUnitsImpl.java

+5-7
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,15 @@
2121
import java.util.List;
2222
import java.util.Properties;
2323

24+
import org.apache.hadoop.fs.FileSystem;
25+
import org.apache.hadoop.fs.Path;
26+
2427
import com.google.api.client.util.Lists;
2528
import com.google.common.io.Closer;
2629

2730
import io.temporal.failure.ApplicationFailure;
28-
2931
import lombok.extern.slf4j.Slf4j;
3032

31-
import org.apache.hadoop.fs.FileSystem;
32-
import org.apache.hadoop.fs.Path;
33-
3433
import org.apache.gobblin.commit.DeliverySemantics;
3534
import org.apache.gobblin.converter.initializer.ConverterInitializerFactory;
3635
import org.apache.gobblin.destination.DestinationDatasetHandlerService;
@@ -45,10 +44,9 @@
4544
import org.apache.gobblin.source.workunit.WorkUnit;
4645
import org.apache.gobblin.source.workunit.WorkUnitStream;
4746
import org.apache.gobblin.temporal.ddm.activity.GenerateWorkUnits;
48-
import org.apache.gobblin.temporal.ddm.work.assistance.Help;
4947
import org.apache.gobblin.temporal.ddm.util.JobStateUtils;
48+
import org.apache.gobblin.temporal.ddm.work.assistance.Help;
5049
import org.apache.gobblin.temporal.workflows.metrics.EventSubmitterContext;
51-
import org.apache.gobblin.util.ConfigUtils;
5250
import org.apache.gobblin.writer.initializer.WriterInitializerFactory;
5351

5452

@@ -70,7 +68,7 @@ public int generateWorkUnits(Properties jobProps, EventSubmitterContext eventSub
7068
// jobState.setBroker(jobBroker);
7169
// jobState.setWorkUnitAndDatasetStateFunctional(new CombinedWorkUnitAndDatasetStateGenerator(this.datasetStateStore, this.jobName));
7270

73-
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(jobProps));
71+
AutomaticTroubleshooter troubleshooter = AutomaticTroubleshooterFactory.createForJob(jobProps);
7472
troubleshooter.start();
7573
try (Closer closer = Closer.create()) {
7674
// before embarking on (potentially expensive) WU creation, first pre-check that the FS is available

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/ProcessWorkUnitImpl.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import org.apache.gobblin.temporal.ddm.util.JobStateUtils;
5757
import org.apache.gobblin.temporal.ddm.work.WorkUnitClaimCheck;
5858
import org.apache.gobblin.temporal.ddm.work.assistance.Help;
59-
import org.apache.gobblin.util.ConfigUtils;
6059
import org.apache.gobblin.util.JobLauncherUtils;
6160

6261

@@ -76,7 +75,7 @@ public int processWorkUnit(WorkUnitClaimCheck wu) {
7675
List<WorkUnit> workUnits = loadFlattenedWorkUnits(wu, fs);
7776
log.info("{} - loaded; found {} workUnits", correlator, workUnits.size());
7877
JobState jobState = Help.loadJobState(wu, fs);
79-
troubleshooter = AutomaticTroubleshooterFactory.createForJob(ConfigUtils.propertiesToConfig(jobState.getProperties()));
78+
troubleshooter = AutomaticTroubleshooterFactory.createForJob(jobState.getProperties());
8079
troubleshooter.start();
8180
return execute(workUnits, wu, jobState, fs, troubleshooter.getIssueRepository());
8281
} catch (IOException | InterruptedException e) {

0 commit comments

Comments
 (0)