Abort enqueue when the concurrency limit is reached

[TAGraves]

Curious to get your feedback on this!

We implemented concurrency keys in some of our jobs for the first time today, and I noticed that when a job isn't enqueued because the concurrency limit is reached, Rails still logs a line like

Enqueued TestJob (Job ID: 39afef71-ad97-4a53-adad-2ba05636d440) to (default) with arguments: {:name=>"Alice"}

This happens because ActiveJob logs as long as enqueue didn't raise an error or get aborted. Maybe the best fix actually lies there although it's not totally clear to me how best to patch that.

Anyway, this PR uses before_enqueue instead of around_enqueue and throws :abort when a job won't be enqueued. That instead causes Rails to log

Failed enqueuing TestJob to (default), a before_enqueue callback halted the enqueuing execution.

which is more accurate, although I think Failed is scary language there. So I also added a log like

Aborting enqueue of TestJob (Job ID: 39afef71-ad97-4a53-adad-2ba05636d440) because the concurrency key 'Alice' has reached its limit of 1 job

to help make it more clear what's going on.

[bensheldon]

@TAGraves I like this. I think it will need some Rails version-compatibility patches because I think the throw(:abort) only works with Rails 6.1+ (I think!)

Another alternative is to try raising an ActiveJob::EnqueueError, though that's only compatible with Rails 7.0+ (I think!).

I'm imagining that this might need to be something like this (it could also maybe be done with feature detection rather than version comparisons, maybe):

if Gem::Version.new(ActiveJob::VERSION) >= Gem::VERSION.new(7.0.0)
  before_enqueue do |job|
    _check_enqueue_concurrency(job, on_abort: :raise)
  end
elsif Gem::Version.new(ActiveJob::VERSION) >= Gem::VERSION.new(6.1.0)
  before_enqueue do |job|
    _check_enqueue_concurrency(job, on_abort: :throw)
  end
else
  around_enqueue do |job, block|
    _check_enqueue_concurrency(job, on_abort: block)
  end
end

[TAGraves]

@bensheldon That all makes sense to me. I think I should have some time tonight to work on this, so I'll give it a try then!

[TAGraves]

@bensheldon I took a look at using ActiveJob::EnqueueError, but it looks to me like that only gets rescued from the job's enqueue method, not from the enqueue callback hooks. (At least when I raised it where I'm currently throwing an exception, it got bubbled up all the way to the spec.) It's a shame though, since that would allow us to provide the reason without having to log out a message ourselves. I'm going to move forward with throwing for >= 6.1 and otherwise not throwing.

[TAGraves]

@bensheldon I've updated this PR to switch the strategy based on the ActiveJob -- let me know if anything more needs to be done here! Thanks!

[bensheldon]

@TAGraves thank you! 🙌🏻 This looks fantastic. I especially like the arguments handling both before_ and around_ callback signatures; very nice! When CI is green I'll merge and release.