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

Made Delayed::Backend::ActiveRecord::Job class load lazily #245

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

willnet
Copy link
Contributor

@willnet willnet commented Jan 20, 2025

In Rails, classes like ActiveRecord::Base are designed to be loaded lazily. If a gem eagerly loads classes when required, as this gem did, it can cause issues where configurations written in config/initializers/*.rb for ActiveRecord are not applied properly. To avoid this, the ActiveSupport.on_load(:active_record) do ... end block is used to delay loading the ActiveRecord::Base class and Delayed::Backend::ActiveRecord::Job class until after Rails has fully initialized.

In this commit, I wrapped the existing Delayed::Backend::ActiveRecord::Job class definition in a block to minimize changes. However, it might be better to separate Delayed::Backend::ActiveRecord::Configuration into a dedicated file for easier management.

In Rails, classes like `ActiveRecord::Base` are designed to be loaded lazily. If a gem eagerly loads classes when required, as this gem did, it can cause issues where configurations written in `config/initializers/*.rb` for ActiveRecord are not applied properly. To avoid this, the `ActiveSupport.on_load(:active_record) do ... end` block is used to delay loading the `ActiveRecord::Base` class and `Delayed::Backend::ActiveRecord::Job` class until after Rails has fully initialized.

In this commit, I wrapped the existing `Delayed::Backend::ActiveRecord::Job` class definition in a block to minimize changes. However, it might be better to separate `Delayed::Backend::ActiveRecord::Configuration` into a dedicated file for easier management.
Rubocop issued warnings, so I resolved it by splitting each class into its own file.

```
lib/delayed/backend/active_record.rb:33:7: C: Metrics/BlockLength: Block has too many lines. [122/25]
      ActiveSupport.on_load(:active_record) do ...
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/delayed/backend/active_record.rb:36:9: W: Lint/ConstantDefinitionInBlock: Do not define constants this way within a block.
        class Job < ::ActiveRecord::Base ...
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
Fix the following offenses using RuboCop’s autocorrect feature.

```
Offenses:

lib/delayed/backend/active_record/job.rb:55:15: C: [Correctable] Layout/MultilineMethodCallIndentation: Align .min_priority with ready_to_run(worker.name, max_run_time) on line 54.
              .min_priority
              ^^^^^^^^^^^^^
lib/delayed/backend/active_record/job.rb:56:15: C: [Correctable] Layout/MultilineMethodCallIndentation: Align .max_priority with ready_to_run(worker.name, max_run_time) on line 54.
              .max_priority
              ^^^^^^^^^^^^^
lib/delayed/backend/active_record/job.rb:57:15: C: [Correctable] Layout/MultilineMethodCallIndentation: Align .for_queues with ready_to_run(worker.name, max_run_time) on line 54.
              .for_queues
              ^^^^^^^^^^^
lib/delayed/backend/active_record/job.rb:58:15: C: [Correctable] Layout/MultilineMethodCallIndentation: Align .by_priority with ready_to_run(worker.name, max_run_time) on line 54.
              .by_priority
              ^^^^^^^^^^^^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant