[ 
https://issues.apache.org/jira/browse/GOBBLIN-1302?focusedWorklogId=504919&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-504919
 ]

ASF GitHub Bot logged work on GOBBLIN-1302:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 26/Oct/20 21:12
            Start Date: 26/Oct/20 21:12
    Worklog Time Spent: 10m 
      Work Description: 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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 504919)
    Time Spent: 50m  (was: 40m)

> make CombineRetentionPolicy easily configurable
> -----------------------------------------------
>
>                 Key: GOBBLIN-1302
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1302
>             Project: Apache Gobblin
>          Issue Type: Improvement
>            Reporter: Arjun Singh Bora
>            Priority: Major
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> currently these configs need to have config prefixed by 
> `combine.retention.policy.class.`
> and each policy need a separate config. we should make one config defining 
> all the individual retention policies



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to