[GitHub] [gobblin] Will-Lo commented on a diff in pull request #3719: [GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl…

["Will-Lo (via GitHub)" <gi...@apache.org>]

Will-Lo commented on code in PR #3719:
URL: https://github.com/apache/gobblin/pull/3719#discussion_r1269670013


##########
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/HelixJobsMapping.java:
##########
@@ -96,15 +96,17 @@ public HelixJobsMapping(Config sysConfig, URI fsUri, String rootDir) {
   }
 
   public static String createPlanningJobId (Properties jobPlanningProps) {
+    long planningJobId = PropertiesUtils.getPropAsBoolean(jobPlanningProps, GobblinClusterConfigurationKeys.USE_GENERATED_JOBEXECUTION_IDS, "false") ?
+        System.currentTimeMillis() : PropertiesUtils.getPropAsLong(jobPlanningProps, ConfigurationKeys.FLOW_EXECUTION_ID_KEY, System.currentTimeMillis());

Review Comment:
   The advantage is primarily for tracking, GobblinTrackingEvents have an execution ID in their metadata and it would work nicely if the execution ID also allowed the caller to identify the flow execution ID from it as well, lets the user perform cancel() etc.
   Uhh for earlyStop maybe we can append something to the jobName but it'll be hacky, I feel like that feature should be redesigned in general.



-- 
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: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org