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


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagActionReminderScheduler.java:
##########
@@ -128,23 +128,27 @@ public void execute(JobExecutionContext context) {
   }
 
   /**
-   * Creates a key for the reminder job by concatenating all dagAction fields
+   * Creates a key for the reminder job by concatenating all dagAction fields 
and the eventTime of the dagAction to
+   * allow reminders for actions associated with multiple flow executions 
within a deadline period (e.g. another
+   * flow execution may occur before a flow finish or job start deadline 
expires)

Review Comment:
   this doesn't capture the nuance of why incorporating the flowExecId into the 
key (as already being done) is insufficient and an event timestamp is needed in 
addition.
   
   please explain...  (doesn't it have to do w/ event consolidation?)



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