rails / solid_queue

Database-backed Active Job backend
MIT License
1.96k stars 131 forks source link

Recurring schedule hides configuration errors #358

Open majkelcc opened 2 months ago

majkelcc commented 2 months ago

When configuring recurring jobs I've noticed that SolidQueue is hiding configuration mistakes that make it difficult to catch potential errors:

  1. Jobs that don't pass SolidQueue::RecurringTask's validation are silently filtered out without any error
  2. Setting SOLID_QUEUE_RECURRING_SCHEDULE to a path that doesn't exist makes SolidQueue silently fall back to an empty schedule

Expected behaviour: above errors should trigger an exception.

majkelcc commented 2 months ago

I also noticed a very odd behaviour:

rosa commented 2 months ago

when I pass an invalid path via SOLID_QUEUE_RECURRING_SCHEDULE then SolidQueue defaults to the default config/recurring.yml configuration file

Hmm... I'm not seeing this. When I pass an invalid path via SOLID_QUEUE_RECURRING_SCHEDULE, the default path in config/recurrring.yml is not used either. The code path for both options (environment variable or CLI option) is the same, so it's strange the behaviour would be different.

majkelcc commented 2 months ago

I thought I'm just tired, but I can still replicate this behaviour - I'm testing it by adding a new task to config/recurrring.yml - it gets added every time when I'm adding SOLID_QUEUE_RECURRING_SCHEDULE=non_existing_path.yml, doesn't when using --recurring_schedule_file=non_existing_path.yml.

Also this will no longer be an issue when passing an invalid path in any of these methods will result in an error.

rosa commented 2 months ago

Could you copy the full command you're using to run Solid Queue in each case?

majkelcc commented 2 months ago

Used commands:

Now I made a syntax error in the default config/recurring.yml and discovered that when using the env variable, SolidQueue is always reading that default configuration file, even when the env variable is pointing to a valid alternative configuration file. When using --recurring_schedule_file the default configuration file is not read and the syntax error there is not producing any errors.

rosa commented 2 months ago

What version are you running? What you describe sounds a lot like what was fixed in https://github.com/rails/solid_queue/pull/345 🤔 This would have been included in 1.0.0.beta.

majkelcc commented 2 months ago

I'm on 0.9.0 😅

Indeed the env variable seems to be simply ignored by cli, I got confused by the combination of these few issues.

rosa commented 2 months ago

Ahhhh! So sorry about that! It should be fixed now 😅

About the issue with the configuration, yes, it's on my radar; I've got it in my plans to raise or print some warnings about invalid configuration.

majkelcc commented 2 months ago

About the issue with the configuration, yes, it's on my radar; I've got it in my plans to raise or print some warnings about invalid configuration.

Raise please 😅

Also reading the code I could see that recurring.yml supports nesting tasks under rails-env keys (production, staging, etc.) - knowing this would save me some time. It might be quite a common thing to have recurring tasks tied to specific environments, might make sense to add this example in the docs ✌️

rosa commented 2 months ago

Also reading the code I could see that recurring.yml supports nesting tasks under rails-env keys (production, staging, etc.) - knowing this would save me some time.

Hmm... I think this is standard for all Rails configurations 🤔 But yes, I'll enhance the example with this.