phet commented on code in PR #3771:
URL: https://github.com/apache/gobblin/pull/3771#discussion_r1325136912
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -496,7 +496,9 @@ public void runJob(Properties jobProps, JobListener
jobListener) throws JobExcep
ConfigurationKeys.ORCHESTRATOR_TRIGGER_EVENT_TIME_NEVER_SET_VAL);
this.orchestrator.orchestrate(flowSpec, jobProps,
Long.parseLong(triggerTimestampMillis));
} catch (Exception e) {
- throw new JobException("Failed to run Spec: " +
jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY), e);
+ String exceptionPrefix = "Failed to run Spec: " +
jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY);
+ log.warn(exceptionPrefix + " with stacktrace", e);
Review Comment:
nit: "with stacktrace" is fine, but "because" might read better
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -544,6 +553,7 @@ static class GetEventInfoResult {
*/
@Data
static class SelectInfoResult {
+ public static final long LEASE_COMPLETED_VALUE = -1;
Review Comment:
how about an `Optional` instead?
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiterTest.java:
##########
@@ -17,8 +17,10 @@
package org.apache.gobblin.runtime.api;
+import com.google.common.base.Optional;
Review Comment:
let's use java `Optional`
(seems the original class under test mistakenly used the deprecated guava
one... I presume it's not too difficult at this early stage to correct)
--
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]
