Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
pawelkaczor opened this issue Apr 30, 2020 · 5 comments · Fixed by pawelkaczor/akka-quartz-scheduler#1 · May be fixed by #110
Open

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

pawelkaczor opened this issue Apr 30, 2020 · 5 comments · Fixed by pawelkaczor/akka-quartz-scheduler#1 · May be fixed by #110

Comments

@pawelkaczor
Copy link
Contributor

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
Copy link
Owner

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

pawelkaczor added a commit to pawelkaczor/akka-quartz-scheduler that referenced this issue May 10, 2020
@felipebonezi
Copy link
Contributor

@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
Copy link
Contributor Author

pawelkaczor commented Jun 12, 2021

@felipebonezi Please see discussion: #94

I created new PR (the same implementation as in previous, rejected PR): #110

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

@pawelkaczor pawelkaczor linked a pull request Jun 12, 2021 that will close this issue
@enragedginger enragedginger reopened this Jun 15, 2021
@enragedginger
Copy link
Owner

@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
Copy link
Contributor Author

@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'"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants