Will-Lo commented on code in PR #4023:
URL: https://github.com/apache/gobblin/pull/4023#discussion_r1712907444
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -412,20 +411,19 @@ protected void logNewlyScheduledJob(JobDetail job,
Trigger trigger) {
/**
* Unschedule and delete a job.
+ * Returns true is job was scheduled and is successfully unscheduled, false
otherwise
*
* @param jobName Job name
* @throws JobException when there is anything wrong unschedule the job
*/
- public void unscheduleJob(String jobName)
+ public boolean unscheduleJob(String jobName)
Review Comment:
I think for the sake of swallowing exceptions it's trading changing a class
function signature to move the log to info. Is it possible instead just to
either ignore if the scheduledJobs does not contain the key? Seems that's the
RC of the noisy logs where it would delete a job that does not exist in the
cache.
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -617,23 +616,17 @@ public AddSpecResponse onAddSpec(Spec addedSpec) {
* @param specURI
* @param specVersion
*/
- private void unscheduleSpec(URI specURI, String specVersion) throws
JobException {
+ private boolean unscheduleSpec(URI specURI, String specVersion) throws
JobException {
if (this.scheduledFlowSpecs.containsKey(specURI.toString())) {
_log.info("Unscheduling flowSpec " + specURI + "/" + specVersion);
this.scheduledFlowSpecs.remove(specURI.toString());
this.lastUpdatedTimeForFlowSpec.remove(specURI.toString());
- unscheduleJob(specURI.toString());
- try {
- FlowSpec spec = this.flowCatalog.get().getSpecs(specURI);
- Properties properties = spec.getConfigAsProperties();
- _log.info(jobSchedulerTracePrefixBuilder(properties) + "Unscheduled
Spec");
- } catch (SpecNotFoundException e) {
- _log.warn("Unable to retrieve spec for URI {}", specURI);
- }
+ return unscheduleJob(specURI.toString());
} else {
- throw new JobException(String.format(
+ _log.info(String.format(
Review Comment:
Small nit: May be should be one word
--
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]
