Fix before/after_schedule hooks not called when enqueued with config#681
Fix before/after_schedule hooks not called when enqueued with config#681jweir wants to merge 4 commits intoresque:masterfrom
Conversation
https://github.com/resque/resque-scheduler#hooks enqueue_at works as documented, but schedules enqueued from a config have not been calling the before/after schedule hooks. fixes resque#679
iloveitaly
left a comment
iloveitaly
left a comment
There was a problem hiding this comment.
@jweir this looks like a great change! Could you:
- Explain exactly what this changes a bit more? Why it was needed?
- Submit a changelog entry
- Rebase on master so CI runs
Would love to get this merged in!
|
@iloveitaly hi there (I came across this after finding #679) I've got a scheduled job that needs to behave differently (not enqueue as configured, but kick off a separate class of the job instead) depending on some runtime variables (eg. feature flag). The |
|
@PatrickTulskie (seeing as this PR is quite out of date, wanted to get it back on the radar for consideration) 🙏 |
|
@codealchemy Happy to get this one merged in since it does seem like it fixes an actual bug. It would be nice to get this branch rebased with master so that CI runs for it. I can handle the changelog updates when I do a release. Also possible that given how old this is, the PR might have to be adjusted. If you want to open a fresh PR and take over work on it, then go for it. |
|
Thanks @PatrickTulskie - I've rebased these changes and opened #792, failing builds there appear unrelated to the changes (and the same as seen on |
|
@codealchemy Yeah I've seen those failures too. Something about the test matrix here needs to get fixed I guess. I'll look at that and your PR and see what I can do to get a release done. |
|
@PatrickTulskie thanks for looking into it, I've taken a swing at addressing the CI failures in a couple PRs (#793, #794) - here's a green CI with the three changes together. |
|
@PatrickTulskie please let me know if there's anything else I can do to help this along 🙇 |
Will do. I'm currently neck deep in 2 different things that are taking up all of my time. I just went through and teed up this and a bunch of other things that need my review though and I'm hoping to get through them all in the next week or two when I have a little down time. |
|
Closing. This will get handled here: #792 |
5 participants
No description provided.