search

enragedginger / akka-quartz-scheduler

Quartz Extension and utilities for cron-style scheduling in Akka
Other
559 stars 115 forks source link

Improve 'Setting up scheduled job ...' log message #93

Open pawelkaczor opened 4 years ago

pawelkaczor commented 4 years ago

Currently the following message is logged (for each schedule entry):

12:24:09,037 INFO [QuartzSchedulerExtension] Setting up scheduled job 'TestJob', with 'com.typesafe.akka.extension.quartz.QuartzCronSchedule@35876530'

The message is not very useful.

In my project I produce the following log message:

12:24:09,035 INFO [PeriodicJobScheduling] Job 'TestJob' will be triggered at 03:01, timezone: Europe/Berlin, calendar: not defined

that contains cron expression description, timezone and calendar (if defined).

enragedginger commented 4 years ago

@pawelkaczor Feel free to submit a PR for this and I'll look at it.

felipebonezi commented 3 years ago

@pawelkaczor I saw that you've commited your suggestion about one year ago.

Dis you finished? I think it would be relevant for the community.

pawelkaczor commented 3 years ago

@felipebonezi Please see discussion: https://github.com/enragedginger/akka-quartz-scheduler/pull/94

I created new PR (the same implementation as in previous, rejected PR): https://github.com/enragedginger/akka-quartz-scheduler/pull/110

@enragedginger please reopen this issue. It was closed by mistake.

enragedginger commented 3 years ago

@felipebonezi Per my review on #94 I still don't think we should merge this (#110) in. As @pawelkaczor mentioned in the PR, merging this PR results in the addition of a number of new dependencies. This will result in unnecessary bloat for our users and potential headaches if they require different versions of these dependencies. Joda and commons-lang are very common, so I'd be surprised if no one ran into conflicts here.

Supposing users want to print out cron expressions for their jobs in some readable format, there are a number of other options that exist. Most promising is that the cron expression exists on the QuartzCronSchedule class. So folks can just pull that from the schedule map and print it at will. This requires no further changes if I'm not mistaken.

If for some reason that doesn't work, then perhaps we could add a general postSchedule hook of sorts which would be an arbitrary function that is run on scheduling.

More generally though, we could update the library to take an Option[ActorRef] on startup that serves as an event bus of sorts. Any time our scheduler code does anything interesting (adds a job, removes a job, fires a job, etc) we would publish an event to this bus and let an Option[ActorRef] somewhere else in the system handle it.

@felipebonezi @pawelkaczor What are your thoughts on this?

pawelkaczor commented 3 years ago

@enragedginger Publishing events seems as a good idea, but IMO is quite a different story.

Here we are dealing with a concrete and simple issue - the log message:

12:24:09,037 INFO [QuartzSchedulerExtension] Setting up scheduled job 'TestJob', with 'com.typesafe.akka.extension.quartz.QuartzCronSchedule@35876530'

And I'm offering a solution that solves this problem very well (from a "user" perspective). But of course your concerns regarding incorporation of new dependencies are valid! That's why I suggested to create a new module.

Anyway, IMO we should concentrate on fixing the log message.

The minimal solution is the removal of " with 'com.typesafe.akka.extension.quartz.QuartzCronSchedule@35876530'"