Conversation
ebb8716 to
ebd9629
Compare
|
Hi @rosa 👋, could you please take a look at this PR when you have a moment? Thanks so much! |
|
Hey @cupatea, thanks for this! It's a good start, but it needs a few changes. The main ones are:
And then some other more specific changes that I'll note in the code. |
lib/solid_queue/configuration.rb
Outdated
|
|
||
| def invalid_tasks | ||
| recurring_tasks.select(&:invalid?) | ||
| static_recurring_tasks.select(&:invalid?) |
There was a problem hiding this comment.
This doesn't need to change names. It's clear the tasks here are static since this comes from the recurring.yml configuration. We don't need to rename anything here.
lib/solid_queue/scheduler.rb
Outdated
| recurring_schedule.update_scheduled_tasks.tap do |updated_tasks| | ||
| if updated_tasks.any? | ||
| process.update_columns(metadata: metadata.compact) | ||
| end |
There was a problem hiding this comment.
This code is mixing actions at very different levels, making it aware of details it shouldn't need to know, like how to update the metadata for its registered process record, or whether the recurring schedule changed. It should change, perhaps to something like
recurring_schedule.reload!
if recurring_schedule.changed?
refresh_registered_process
endAnd refresh_registered_process would go in SolidQueue::Processes::Registrable.
There was a problem hiding this comment.
Hi @rosa do you think this Dynamic scheduled tasks will merge by the end of this year 2026? thanks
ad943cc to
6a883a7
Compare
|
Hi @rosa, thank you for the feedback! I think it's ready for the second round of review |
6a883a7 to
c754746
Compare
c754746 to
214a7f6
Compare
|
@cupatea , in the docs files you mention |
Thanks for noticing, on a way to fix that! |
017c34e to
dd72f10
Compare
dd72f10 to
62bfa37
Compare
Two assertions were using assert value, message instead of assert_equal/assert_empty, meaning they always passed regardless of the actual metadata content. Fixed to use assert_empty. Also fixed a typo ("unschedule" -> "unscheduled").
DB queries in reload! (dynamic_tasks.where.not(...), RecurringTask.pluck(:key)) were not wrapped in the app executor, which could cause connection management issues. Wrapped in wrap_in_app_executor.
empty? checked stale configured_tasks (lib/solid_queue/scheduler/recurring_schedule.rb) -- configured_tasks was set once in initialize and never updated with dynamic tasks. This meant empty? could return true even when dynamic tasks existed, causing the scheduler to exit prematurely in inline mode. Changed empty? to check scheduled_tasks.empty? && dynamic_tasks.none?.
Changes not cleared after consumption (lib/solid_queue/scheduler.rb, lib/solid_queue/scheduler/recurring_schedule.rb) -- Added clear_changes method and call it in the scheduler's run loop after refresh_registered_process, preventing stale change state from persisting.
Remove extra blank lines, column-aligned hashes, and inline comments on test assertions. Revert unrelated Gemfile.lock platform addition. Filter :static from schedule_task options since dynamic tasks are always non-static by definition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Dynamic tasks require explicit `dynamic_tasks: true` in the scheduler config. Without it, the scheduler behaves as before — no extra DB queries, no polling for dynamic changes. Default polling interval changed from 1s to 5s. Rename schedule_task/unschedule_task to schedule_recurring_task/ unschedule_recurring_task so the API clearly communicates these are recurring tasks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tasks_enabled Have from_configuration read static from options instead of hardcoding it. Callers that create static tasks (YAML config) pass static: true via reverse_merge. schedule_recurring_task passes static: false directly. Rename the config key from dynamic_tasks to dynamic_tasks_enabled for clarity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep the longer sleep interval when dynamic tasks are disabled. Add tests for `dynamic_tasks_enabled` opt-in in configuration and for verifying dynamic tasks are ignored when not enabled.
We don't need to track dynamic task changes, we can just reload the metadata in every case. If it hasn't changed, Rails won't issue any new update to the process record. Also, we can just use `dynamic_tasks` everywhere, as an empty AR relation if dynamic tasks are disabled, avoiding extra queries but keeping the code simpler.
62bfa37 to
b4f7c99
Compare
|
Hey @cupatea, sorry for the delay here. I'm finally working towards Solid Queue 2.0 with the plan of including this. I've pushed a bunch of changes. Some are stylistic and simplifications, but I've also changed the default behaviour. I think this feature should be opt-in, disabled by default, as people might not need dynamic tasks at all and could save the extra queries to the DB. |
We need the dynamic task keys there and were doing a new query every time. We only need to do a query when explicitly reloading them.
Thank you! I noticed something in |
Ohh, totally! My bad. I had it backwards before, switched it around, but forgot to update the method. Thanks! |
We want to set that value overriding whatever we get in options, so it should be merged into options, not the other way around. Co-Authored-By: Vladyslav Davydenko <vladyslav@hey.com>
Not really - I migrated from Resque to this and deployed to the test server, but I haven’t deployed it to production yet. |
Fixes #186
Add resque-scheduler style dynamic schedules feature, allowing you to add or remove recurring tasks at runtime without touching your static config file.
What’s new:
SolidQueue::RecurringTaskmodel.SolidQueue::Scheduler::RecurringScheduleto distinguish static vs. dynamic schedules.@configured_tasksnow includes static and dynamic tasks.SolidQueue::Scheduler::RecurringSchedule.update_scheduled_tasks:SolidQueue::Configurationno longer requires a non-blank static config file - pure dynamic scheduling is now supported.SolidQueue::Schedulerwatches for changes after launch and updates its metadata so the running process always reflects the true set of recurring tasks.Tests verify that adding or dropping dynamic tasks at runtime correctly updates what’s scheduled.