sv2000 commented on a change in pull request #3140:
URL: https://github.com/apache/incubator-gobblin/pull/3140#discussion_r512267477
##########
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/retention/policy/CombineRetentionPolicy.java
##########
@@ -110,6 +105,30 @@ public CombineRetentionPolicy(Properties props) throws
IOException {
}
+ private List<RetentionPolicy<T>> findRetentionPolicies(Properties props) {
+ List<String> retentionPolicyClasses;
+ ImmutableList.Builder<RetentionPolicy<T>> builder =
ImmutableList.builder();
+ ClassAliasResolver<?> aliasResolver = new
ClassAliasResolver<>(RetentionPolicy.class);
+
+ if (props.containsKey(COMBINE_RETENTION_POLICIES)) {
+ retentionPolicyClasses =
COMMA_BASED_SPLITTER.splitToList(props.getProperty(COMBINE_RETENTION_POLICIES));
Review comment:
Ack. Thanks!
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/HelixUtils.java
##########
@@ -204,7 +204,7 @@ static void waitJobCompletion(HelixManager helixManager,
String workFlowName, St
} else {
// We have waited for WorkflowContext to get initialized,
// so it is found null here, it must have been deleted in job
cancellation process.
- log.info("WorkflowContext not found. Job is probably cancelled.");
+ log.info("WorkflowContext not found. Job {} is probably cancelled.",
jobName);
Review comment:
Even though it is a trivial one-line change, I prefer to keep unrelated
changes in separate PRs. The context of such unrelated changes is lost after
some time and can be confusing to a new developer. My suggestion is to pull
this change out of this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
