Scheduler.exists(TaskInstanceId)

[zeitwesen]

I'm using your library to fulfill a timeout-watchdog. So basicly I use OneTimeTasks that timeout unless reset (canceled and rescheduled) before.
Currently I'm catching the RuntimeException of scheduler.cancel and schedule the task again as follows:

try {
	scheduler.cancel(TaskInstanceId.of(tasktype.getTaskName(), watchdogId.getId()));
} catch (RuntimeException e) {
	if (e.getMessage().startsWith("Could not cancel schedule")) {
		LOG.debug("Exception:", e);
		return;
	}
	throw e;
}
// and afterwards schedule the task again

What I'm looking for is an exists-Method to get rid of this ugly exception handling, e.g.:

	if(scheduler.exists(instanceId)){
		scheduler.cancel(instanceId);
	}
	scheduler.schedule(...);

[kagkarlsson]

Hi!
I think you can use this existing method on the scheduler/scheduler-client:

getScheduledExecutionsForTask(String taskName, Class<T> dataClass, Consumer<ScheduledExecution<T>> consumer)

It will not fetch a specific instance of the task, but if there is only one instance of the task scheduled, you can check it in your code.

Let me know if this does not solve your use-case :)

[zeitwesen]

Thanks for your quick answer!
I just digged into your proposal and probably found two bugs as it does not work the way I expected it:

• your getScheduledExecutionsForTask(..) method does not delegate the taskName to the query, so it queries all tasks
• your getScheduledExecutionsForTask(..) method does not call the consumer, so it is basicly never called / useless :)

Can you reconfirm my findings?

Class: SchedulerClient
@Override
		public <T> void getScheduledExecutionsForTask(String taskName, Class<T> dataClass, Consumer<ScheduledExecution<T>> consumer) {
			taskRepository.getScheduledExecutions(execution -> new ScheduledExecution<>(dataClass, execution));
		}

[kagkarlsson]

Hah, good catch, that indeed looks like a bug! In practise, String taskName is never used.

The other one seems ok though? (See test)

I have never used this functionality myself, it was PR-ed in. I will fix the bug though.

[kagkarlsson]

So yeah, the method getScheduledExecutionsForTask(String taskName, Class<T> dataClass, Consumer<ScheduledExecution<T>> consumer) is broken, but

getScheduledExecutions(Consumer<ScheduledExecution<Object>> consumer) should work

[zeitwesen]

yep, just checked it again, in SchedulerClient, Line 118 (see snipped above) the consumer is also not passed on or called - so it is basicly unused. Your mentioned test seems to just cover the underlying TaskRepository.

I will stay with my Exception-Handler by now and see if you manage to provide a bugfix some time. Thanks again for providing this marvellous library :)

[kagkarlsson]

Thanks for the feedback. Will get this issue fixed asap :)

[zeitwesen]

No hurry.

Maybe you could also provide the exists(instanceId) feature :)

[kagkarlsson]

Btw, what about the reschedule-method
reschedule(TaskInstanceId taskInstanceId, Instant newExecutionTime)

[kagkarlsson]

Ah, no, you must handle exceptions in that case aswell.

[zeitwesen]

yep, just gave it a try:
java.lang.RuntimeException: Could not reschedule - no task with name 'XXXXXXXX' and id 'XXXXXXXXXXXX' was found.

But no hurry - so far I can live with my handled .cancel()method.

[kagkarlsson]

PR #42 has a fix for the getScheduledExecutionsForTask()

Will probably implement a method Optional<ScheduledExecution> Scheduler.get(TaskInstanceId) instead of an exists method.

[kagkarlsson]

The fix is now available in version 4.1

[zeitwesen]

The fix is now available in version 4.1

cool, thanks for that. Will give it a try later and provide you feedback.

[zeitwesen]

Just tried it -> works like a charme! Thanks for the quick fix!

TaskInstanceId taskInstanceId = TaskInstanceId.of(taskName, id);
		scheduler.getScheduledExecutionsForTask(taskName, String.class, e -> {
			if (e.getTaskInstance().getId().equals(taskInstanceId.getId())) {
				scheduler.cancel(taskInstanceId);
			}
		});

One improvement still: Could you also implement StandardTaskInstanceId.equals(..) and .hashCode() so I do not explicitly have to compare getTaskInstance().getId() to check if it is the same TaskInstance? Now this comparison would yield false.

[kagkarlsson]

PR #53 provides a method getScheduledExecution(instanceId) which will be slightly better for you. Have also added equals and hashCode to StandardTaskInstanceId. To be released soon.
If you have the time, would you take a quick look at the PR and confirm?