phet commented on code in PR #3841:
URL: https://github.com/apache/gobblin/pull/3841#discussion_r1416344671


##########
gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/DagActionStoreChangeMonitor.java:
##########
@@ -192,9 +230,16 @@ protected void processMessage(DecodeableKafkaRecord 
message) {
     dagActionsSeenCache.put(changeIdentifier, changeIdentifier);
   }
 
-  protected void submitFlowToDagManagerHelper(DagActionStore.DagAction 
dagAction) {
+  /**
+   * Used to forward a launch flow action event from the DagActionStore. 
Because it may be a completely new DAG not
+   * contained in the dagStore, we compile the flow to generate the dag before 
calling addDag(), handling any errors
+   * that may result in the process.
+   */
+  protected void submitFlowToDagManagerHelper(DagActionStore.DagAction 
dagAction, boolean isLoadedOnStartup) {

Review Comment:
   given the only thing `isLoadedOnStartup` determines is which meter to 
increment, I recommend to codify that:
   ```
   @Data
   private class SubmissionMetricProxy {
     private final ContextAwareMeter successMeter;
     private final ContextAwareMeter failureMeter;
   
     public static SubmissionMetricProxy ON_STARTUP = new SubmissionMetricProxy(
   DagActionStoreChangeMonitor.this.successfulLaunchSubmissionsOnStartup,
   DagActionStoreChangeMonitor.this.failedLaunchSubmissionsOnStartup);
   
   public static SubmissionMetricProxy POST_STARTUP = ...;
   
     public void markSuccess() {
       getSuccessMeter.mark();
     }
     public void markFailure() {
       getFailureMeter.mark();
     }
   }
   ```
   
   then create an overloaded version:
   ```
   protected void submitFlow(DagAction dagAction, boolean isStartup) {
     submitFlow(dagAction, isStartup ? SubmissionMetricProxy.ON_STARTUP : 
SubmissionMetricProxy.POST_STARTUP);
   }
   
   protected void submitFlow(DagAction dagAction, SubmissionMetricProxy smp) {
     ...
   }
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to