-
Notifications
You must be signed in to change notification settings - Fork 9
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
2 changed files
with
6 additions
and
3 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,6 @@ | |
|
||
module Sidekiq | ||
module Debouncer | ||
VERSION = "2.0.2" | ||
VERSION = "3.0.0" | ||
end | ||
end |
a7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this being a complete re-write, is there an upgrade path/migration that folks need to take? Like, what happens to anything that was enqueued with the "old way?"
a7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the previous version, debouncing was handled directly on
scheduled
set, so all jobs enqueued from 2.x will still be executed normally (although v3 won't debounce jobs enqueued from v2). There are no changes required in the code. Optionally, you can enable web uia7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably an important note to call out in the CHANGELOG, or even in the README for folks upgrading from 2.0 to 3.0. It's likely not such a big deal if you're debouncing high-volume over a short time period. But I know there are folks debouncing a medium/low volume over a longer time - like hours to days!
a7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've confirmed that in at least one case, this is being used to debounce behavior over 2 weeks! The use case is something like sending a reminder notification to a user if they've not reviewed the most recent changes to a document. I think the downside here would be extra notifications, which is probably not the worst thing. But if there's a way to migrate the V2 jobs to be considered when debouncing V3, I'd like to explore it.
What do you think?
a7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add notes to changelog today.
If it comes to migrating jobs from V2 to V3 - I don't see an easy way. The format is completely different. In V2 jobs stay in
scheduled
set and have additional key "pointing" at them, so it's easy to modify them. In V3 we have a completely separated set, with different job format.a7c40be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fair enough. We went through and audited our usages of
sidekiq-debouncer
and while we have some risk of missing a few debounced when upgrading, it's an acceptable risk for us given the kinds of events we're debouncing. But I think at least a note in theCHANGELOG
would be good so others aren't caught off guard.Thanks again for the effort on this!