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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dashboard doesn't warn of misconfigured job class in cron #1261

Open
duncan-bayne opened this issue Feb 22, 2024 · 1 comment
Open

Dashboard doesn't warn of misconfigured job class in cron #1261

duncan-bayne opened this issue Feb 22, 2024 · 1 comment

Comments

@duncan-bayne
Copy link

Hi :) Firstly thanks for GoodJob, it's not just good, it's great 馃挅

Recently, I misconfigured the cron setting for GoodJob with a typo in the class setting:

  config.good_job.cron = {
    MyJob: { cron: "50 * * * *", class: 'MyJobb'},
  }

This "worked" in the sense that the application started up, and the job could be run manually from the Rails console.

However, the job didn't run periodically as intended, but also, no errors or warnings were displayed in the Dashboard.

I'm not sure how this should be handled though. My initial reaction is that GoodJob should just refuse to start in the presence of a bad config like this: i.e. fail early & noisily.

Versions of things:

  • Rails 7.1.3
  • Ruby 3.2.2
  • GoodJob 3.24.0
@bensheldon
Copy link
Owner

Thanks for opening the issue! And sorry you ran into this trouble.

You're correct that GoodJob doesn't validate the configuration; and also if it fails to enqueue any job, there is no record of that to display through the dashboard.

If you have config.good_job.on_thread_error = ... configured it should be reporting the missing constant error for a misnamed job.

The "validate at startup" is tricky because GoodJob tries to minimize its autoloading footprint (that's why the classes are defined as strings so that loading can be deferred).

I'm thinking: maybe there could be a new configuration like config.good_job.cron_validated = ... that could be opted into (though validating Procs would probably be difficult). This also makes me think of #1232 too, because ideally the cron configuration could also be inspected/validated in your application's tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Inbox
Development

No branches or pull requests

2 participants