Betterment / delayed

a multi-threaded, SQL-driven ActiveJob backend used at Betterment to process millions of background jobs per day
MIT License
154 stars 8 forks source link

Minor Rails 6.1 fix, plus autoload Delayed::Job constant #2

Closed smudge closed 3 years ago

smudge commented 3 years ago

/domain @samandmoore @effron /no-platform

This fixes an issue I was seeing on a very slim Rails 6.1 app -- essentially, ActiveSupport.on_load(:active_record) was not triggering at all during rake delayed:work, resulting in a missing constant error. Previous versions of rails seem to load ActiveRecord::Base during boot, but 6.1 seems to have found a way to optimize that step away.

This PR moves the delayed/job.rb class to app/models and converts the gem's Railtie to an Engine, allowing it to autoload files in app/models by convention. In cases where we are not in a Rails app (and Rails::Engine is not defined), we fall back on loading active_record manually and then running a require_relative of the specific job.rb file. I based this on other "rails-engine-optional" gems I've encountered, and confirmed that it works in an irb console.

I also removed require 'delayed/serialization/active_record' because it would already be loaded inside of require 'delayed/yaml_ext'. It also only applies to syck which dates back to Ruby 1.9, and we should consider removing support for it entirely. (It can still be installed via the syck gem, but psych has been the default for many, many years.)

nanda-prbot commented 3 years ago

Needs somebody from @samandmoore and @effron to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

HOW TO: Claim a Review

nanda-prbot commented 3 years ago

Needs somebody from @samandmoore and @effron to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

HOW TO: Claim a Review

nanda-prbot commented 3 years ago

Needs somebody from @samandmoore and @effron to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

HOW TO: Claim a Review

samandmoore commented 3 years ago

<<domainlgtm

This seems a lil odd to me but it also seems fine. I don't have any better ideas, just ones that are weird in different ways lol

nanda-prbot commented 3 years ago

This PR requires additional review because of new changes

Please get another domain review from @samandmoore, or another reviewer with write access if unavailable.

smudge commented 3 years ago

bump @samandmoore -- ended up updating version+changelog in this PR

samandmoore commented 3 years ago

domainlgtm