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

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

                Author: ASF GitHub Bot
            Created on: 06/May/24 17:09
            Start Date: 06/May/24 17:09
    Worklog Time Spent: 10m 
      Work Description: phet commented on code in PR #3943:
URL: https://github.com/apache/gobblin/pull/3943#discussion_r1591312458


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/proc/LaunchDagProc.java:
##########
@@ -63,8 +63,12 @@ protected Optional<Dag<JobExecutionPlan>> 
initialize(DagManagementStateStore dag
     try {
       FlowSpec flowSpec = 
dagManagementStateStore.getFlowSpec(FlowSpec.Utils.createFlowSpecUri(getDagId().getFlowId()));
       flowSpec.addProperty(ConfigurationKeys.FLOW_EXECUTION_ID_KEY, 
getDagId().getFlowExecutionId());
-      return 
this.flowCompilationValidationHelper.createExecutionPlanIfValid(flowSpec).toJavaUtil();
-    } catch (URISyntaxException | SpecNotFoundException | InterruptedException 
e) {
+      Optional<Dag<JobExecutionPlan>> dag = 
this.flowCompilationValidationHelper.createExecutionPlanIfValid(flowSpec).toJavaUtil();
+      if (dag.isPresent()) {
+        dagManagementStateStore.checkpointDag(dag.get());
+      }
+      return dag;

Review Comment:
   not sure if I'm missing something... why would you return `Optional.empty` 
explicitly?  this illustrates the type of the return instance, which should be 
enough:
   ```
   Optional<Dag<JobExecutionPlan>> dag = 
this.flowCompilationValidationHelper.createExecutionPlanIfValid(flowSpec).toJavaUtil();
   ```
   
   something like this just adds tautological boiler-plate:
   ```
   if (dag.isPresent()) {
     ...
     return dag;
   } else {
     return Optional.empty();
   }
   ```
   is that what you're suggesting?
   
   best analogy I can give is that it would only add useless verbiage in the 
same way as would replacing:
   ```
   return foo > bar;
   ```
   with
   ```
   return foo > bar ? true : false;
   ```





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

    Worklog Id:     (was: 917873)
    Time Spent: 1h 20m  (was: 1h 10m)

> Fix initialization of DagProcessingEngine
> -----------------------------------------
>
>                 Key: GOBBLIN-2061
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-2061
>             Project: Apache Gobblin
>          Issue Type: Bug
>          Components: gobblin-service
>            Reporter: Urmi Mustafi
>            Assignee: Abhishek Tiwari
>            Priority: Major
>          Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> * Initialize DagProcessingEngine with right parameters 
>  * Add DagProcessingEngine to GobblinServiceManager
>  * Add logging to DagProcessingEngine instantiation and Threads
>  * Fix instantiation of QuartzScheduler for DagActionReminderScheduler
>  * Fix DagAction type retrieval from Quartz reminder job detail map
>  *  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to