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

Cron dashboard does not distinguish enabled/disabled jobs #1552

Open
mroach opened this issue Nov 26, 2024 · 4 comments
Open

Cron dashboard does not distinguish enabled/disabled jobs #1552

mroach opened this issue Nov 26, 2024 · 4 comments

Comments

@mroach
Copy link

mroach commented Nov 26, 2024

I'm running Good Job v4.5.0

In my configuration we have jobs that only run in certain environments. Previously we handled this by building the config.good_job.cron hash when the app boots. I recently had a problem that required me to start testing job config. I only just now noticed the relatively new enabled_by_default option. Perfect!

I switched out configuration over and it all works, but the dashboard does not distinguish enabled from disabled jobs and it makes it seem like they're going to execute. Based on my reading of the docs and this issue, it sounds like it should be?

I added a job with enabled_by_default: false and it looks like this:

image

FWIW this is what the config is like. It all works as it should, the only issue is the dashboard.

Rails.application.configure do
  boolean_true_strings = Set["1", "yes", "on", "enabled", "true"]

  config.good_job.enable_cron = true

  config.good_job.cron = {
    noop: {
      cron: "every minute",
      class: "Jobs::Noop",
      description: "No-op test job"
    },
  }.to_h do |key, spec|
    unless spec.key?(:enabled_by_default)
      env_key = "CRON_#{key.to_s.upcase}"
      spec[:enabled_by_default] = -> { boolean_true_strings.include?(ENV[env_key]) }
    end

    [key, spec]
  end
end
@bensheldon
Copy link
Owner

hmmmm. It looks to me like both the dashboard and the cron-functionality itself use CronEntry#enabled?

Are you using the same environment variables injected into the web process that's serving the dashboard as you are the process running cron?

btw, you can use ActiveModel::Type::Boolean.new.cast(boolean_like) to coerce all truthy/falsey values (especially strings from ENV) to booleans.

@mroach
Copy link
Author

mroach commented Nov 26, 2024

Thanks for taking a look! :)

This is in my local development environment where none of the jobs are enabled, so there are no ENV vars set.

I also confirmed this in our staging env where none of the jobs are enabled either and the dashboard looks the same: jobs say they're going to execute in X minutes/hours, but they do not execute (which is right!). So the actual behaviour is correct, it's just that the dashboard is a bit misleading.

Confirmed at the console:

GoodJob::CronEntry.all.any?(&:enabled?)
=> false

To eliminate all the complexity I shared there, here's a much simpler setup that has the same result where the dashboard says they'll execute in a minute.

Rails.application.configure do
  config.good_job.enable_cron = true
  config.good_job.cron = {
    noop1: { cron: "every minute", class: "Jobs::Base", enabled_by_default: false },
    noop2: { cron: "every minute", class: "Jobs::Base", enabled_by_default: -> { false } }
  }
end

image

btw, you can use ActiveModel::Type::Boolean.new.cast(boolean_like) to coerce all truthy/falsey values (especially strings from ENV) to booleans.

Yeahhhh...I'm not a huge fan of this coercer's behaviour. It treats any unknown value as true. I have my own bool-like parser that has an explicit set of true values (like shown) and false (the opposites of all those) and raises an error for anything else. That way there are never any surprises. I have other cases where ENV vars are disabling functionality like USE_NEW_FEATURE=no and that cast method would treat that as true. Granted I could limit config values to ones supported by that coercer, but that still leaves the door open to a typo causing trouble e.g. SOME_FEATURE=offf get cocerced to true. Yikes.

@bensheldon
Copy link
Owner

Ohhhh! I might be overthinking it because it might just be a display problem unrelated to the values themselves. It might be a simple fix to the View

Looking at your screenshot (sorry, haven't had a chance to use code example) I see it shows the "Play" icon, meaning the current status (at least for that column) is disabled.

@mroach
Copy link
Author

mroach commented Nov 26, 2024

Oh man, I didn't even notice that. I definitely knew the >>| icon was to start the job right now and I didn't check the other one. 🤦‍♂️

So that's good to know that that's the UI indicator! :)

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