[
https://issues.apache.org/jira/browse/GOBBLIN-1887?focusedWorklogId=878447&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-878447
]
ASF GitHub Bot logged work on GOBBLIN-1887:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 25/Aug/23 18:39
Start Date: 25/Aug/23 18:39
Worklog Time Spent: 10m
Work Description: umustafi commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306022515
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -181,36 +179,43 @@ private void scheduleReminderForEvent(Properties
jobProps, MultiActiveLeaseArbit
this.jobDoesNotExistInSchedulerCount.inc();
return;
}
- JobKey reminderJobKey = constructReminderJobKey(origJobKey,
status.getEventTimeMillis());
- JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey,
reminderJobKey, cronExpression,
- status.getEventTimeMillis(), originalEventTimeMillis);
- // Create a new trigger that is set to fire at the minimum reminder wait
time calculated
+ JobKey reminderJobKey = constructReminderJobKey(origJobKey, status);
+ JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey,
reminderJobKey, status,
+ triggerEventTimeMillis, schedulerMaxBackoffMillis);
+ // Create a new trigger with a `reminder` suffix that is set to fire at
the minimum reminder wait time calculated
Trigger reminderTrigger =
JobScheduler.createTriggerForJob(reminderJobKey,
- (Properties)
jobDetail.getJobDataMap().get(GobblinServiceJobScheduler.PROPERTIES_KEY),
Optional.absent());
+ getJobPropertiesFromJobDetail(jobDetail),
Optional.of(createSuffixForJobTrigger(status)));
// TODO: remove this comment once we've confirmed this function works
- log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting
to schedule reminder for event {}",
- flowAction, originalEventTimeMillis, status.getEventTimeMillis());
+ log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting
to schedule reminder for event {} with"
+ + "reminderJobKey {} and reminderTriggerKey {}", flowAction,
triggerEventTimeMillis,
+ status.getEventTimeMillis(), reminderJobKey,
reminderTrigger.getKey());
this.schedulerService.getScheduler().scheduleJob(jobDetail,
reminderTrigger);
Review Comment:
Good suggestion, updated!
Issue Time Tracking
-------------------
Worklog Id: (was: 878447)
Time Spent: 20m (was: 10m)
> Increase Encapsulation in FlowTriggerHandler & Add Unit Tests
> -------------------------------------------------------------
>
> Key: GOBBLIN-1887
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1887
> Project: Apache Gobblin
> Issue Type: Bug
> Components: gobblin-service
> Reporter: Urmi Mustafi
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Increase abstraction and encapsulation in FlowTriggerHandler with minor
> refactoring to static helper functions. Unpack objects as late as possible to
> make {{scheduleReminderForEvent}} easier to read. Add unit tests for each of
> the static helper functions. Also ensure that the {{reminderJobKey}} is also
> used as the {{reminderTrigger's key}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
