Repository navigation
Conversation
ca3cfd5 to
5c390c6
Compare
1a48457 to
f4243df
Compare
| def assert_resque_key_exists?(key) | ||
| if Gem::Requirement.create('< 4').satisfied_by?(Gem::Version.create(Redis::VERSION)) | ||
| assert(!Resque.redis.exists(key)) | ||
| else | ||
| assert(!Resque.redis.exists?(key)) | ||
| end | ||
| end |
There was a problem hiding this comment.
Nice. This was around the signature change for exists?, right? https://github.com/redis/redis-rb/blob/master/CHANGELOG.md#420
There was a problem hiding this comment.
Yep thats correct. This was throwing the deprecation warning without this adapter
|
@onyxraven just a heads up, mixing in big unrelated changes to the feature make this tough to approve/merge. Can we break apart the ruby compatibility component from the actual feature here? Dropping support for previous versions of ruby is usually a maintainer decision and since it's bundled together here, it makes the rest of the PR unmergeable at this time. |
Yep you're right. I can definitely split those up. This started out as a private-only fork and I forgot I had pulled all my changes over. I'll update this branch/PR to reflect just the batch opt-out change. |
4fe6f23 to
291fbc2
Compare
| log "Processing delayed items for timestamp #{timestamp}, in batches of #{batch_size}" | ||
| message = "Processing delayed items for timestamp #{timestamp}" | ||
| message += ", in batches of #{batch_size}" if batch_delayed_items? | ||
| log message |
There was a problem hiding this comment.
I tried to keep the log messages consistent for batch vs non-batch
|
|
||
| test 'enqueue_delayed_items_for_timestamp enqueues jobs in 2 batches' do | ||
| t = Time.now + 60 | ||
| context 'non-batch delayed item queue' do |
There was a problem hiding this comment.
new tests for disabling batch
291fbc2 to
34e6cf7
Compare
@PatrickTulskie I've done that. The changeset only has the direct changes now. |
0f29000 to
94ff46e
Compare
|
@onyxraven hey just a heads up, I'm making moves to get through a bunch of the PR backlog for resque scheduler. This looks like a good candidate for 5.1.0. If you have a chance, can you rebase and push this up again and I'll get it live? If you're not interested anymore or I don't hear back from you in the next week or so, I'll have my agent rebase and open a fresh PR with your change so you still get credit for the contribution. |
Can and will do today |
7ff5877 to
4604f09
Compare
6da0c8b to
d311c33
Compare
d311c33 to
8bf62b6
Compare
|
GH is struggling with the build matrix. I moved to minimize my changes to it (I can test older ones locally or with a separate workflow). There are just too many variations. Perhaps need to consider splitting them into multiple workflows or multiple steps if they all need tested (reducing parallelization) |
3 participants
This PR adds a flag that enables/disables the batch enqueue mode for delayed jobs.
The batch enqueue starts a multi/pipeline around the job enqueue for delayed jobs when they are being executed, but this breaks a number of other plugins that rely on doing Redis work within that enqueue window.
The current approach is to make the batch behavior opt-out, with a flag. The documentation is updatd to lay out the compatibility risks.
References:
lantins/resque-lock-timeout#36
#767
#787
#779
#773
This PR also updates compatibility with ruby 3.x. This creates a slightly larger PR, but was required for my use cases and helps to move the gem forward. Some projects choose to make dropping support for old rubies a major version bump, but I'm undecided if thats necessary, and would defer to project maintainers.
Compatibility integration tests with other gems (just resque-lock-timeout for now) exist in this repo - this helps prove out the fix. I hope to keep this other repo up to date with any changes.