Let bin/jobs check validate another env's config section without a database - #790
Open
wintan1418 wants to merge 2 commits into
Open
Let bin/jobs check validate another env's config section without a database#790wintan1418 wants to merge 2 commits into
wintan1418 wants to merge 2 commits into
Conversation
Active Job's retry_on and rescue_from only hook into exceptions raised while perform runs. ProcessPrunedError, ProcessExitError and ProcessMissingError are never raised inside the job: the process running it is already gone, and a different process records the error directly as a failed execution after the fact. People coming from other backends expect retry_on to cover this case and are surprised when it silently doesn't, so spell out why it can't work and point to the mechanisms that do: Mission Control and the fail_many_claimed subscription. Fixes rails#786
…tabase bin/jobs check always read the config files through the current Rails env, so a CI run in the test env couldn't validate the production: section of config/recurring.yml — the one file whose env scoping is load-bearing. Worse, config_from's fallback made it silently validate nothing: with no test: key it took the whole file, treated production as a task with no schedule, dropped it, and reported the configuration valid. A new --env option threads the target env through to both config readers, so `bin/jobs check --env production` validates the section that will actually be deployed. Validation also crashed with a raw StatementInvalid backtrace when the solid_queue tables weren't reachable — instantiating RecurringTask needs its schema, and check already anticipates a missing database a few lines down, in the pool-size warning. Configuration now passes the scheduler raw [key, options] pairs instead of Active Record objects, which RecurringSchedule already knows how to wrap when the scheduler boots, so listing processes needs no database at all. Task-level validation still needs the schema; when it isn't available, check degrades to an explicit warning instead of failing, and still validates everything else. Fixes rails#780
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #780
Follows the analysis in #780 (thanks @kylekeesling for the detailed report — this implements both fixes you outlined).
1.
--envoption forbin/jobs checkcheckalways read the config files through the current Rails env, so a CI run in thetestenv couldn't inspect theproduction:section ofconfig/recurring.yml— andconfig_from's whole-file fallback made it worse: with notest:key it treatedproductionas a task with no:schedule, dropped it, and reported an empty configuration valid.The option only exists on
check—startkeeps running strictly under the current env.2.
checkno longer requires a databaseConfigurationused to instantiateRecurringTaskActive Record models just to list the configured processes, which needs thesolid_queue_recurring_tasksschema and blew up with a rawStatementInvalidbacktrace in environments with no queue database. Now it passes the scheduler raw[ key, options ]pairs —RecurringSchedulealready wraps whatever it receives viaRecurringTask.wrapat boot, in the running process where a database is guaranteed — so process listing is database-free.Task-level validation (schedule syntax, job class existence) still requires the schema to build the models. When there's no usable connection,
checknow degrades to an explicit warning instead of crashing, mirroring the rescue the pool-size warning already had, and still validates everything else:Making task-level validation itself database-free would mean decoupling
RecurringTask's validations from Active Record; that felt like a separate discussion, and the warning keepscheckhonest about what it did and didn't verify in the meantime.Tests
--envtargeting validates theproduction:-only recurring fixture from the test env, including instantiating the scheduler from the raw definitions.RecurringTask.from_configurationraisingStatementInvalid(no schema), the scheduler is still detected from the raw configuration,checkstays valid, and the warning is reported.