Use Sidekiq's API directly (part 1 of 3)#6725
Open
thomasleese wants to merge 3 commits intopds-cascading-job-stringsfrom
Open
Use Sidekiq's API directly (part 1 of 3)#6725thomasleese wants to merge 3 commits intopds-cascading-job-stringsfrom
thomasleese wants to merge 3 commits intopds-cascading-job-stringsfrom
Conversation
thomasleese
force-pushed
the
use-sidekiq-directly-1
branch
5 times, most recently
from
3faefa4 to
ece3423
Compare
thomasleese
force-pushed
the
use-sidekiq-directly-1
branch
6 times, most recently
from
fd3bd16 to
52db5ba
Compare
This converts any of the jobs which take no arguments (and are likely queued on a schedule) to Sidekiq. Jobs without arguments are easier to convert to Sidekiq as there's no need to handle the argument serialisation and deserialisation, therefore we can switch these jobs directly. For `GIASImportJob`, I've removed the `dry_run` parameter as we were never setting this to true. If we needed to run this in a console, we can run the steps manually. Jira-Issue: MAV-7288
This adds Sidekiq-only versions of the `EmailDeliveryJob`, `SMSDeliveryJob` and `NotifyDeliveryJob` jobs, to support the migration to using Sidekiq directly. Using Sidekiq directly doesn't support named parameters in jobs, so we now have to pass a JSON-serializable hash containing the parameters that are passed to the `GovukNotifyPersonalisation` class. I'd like to refactor this further to add safeguards to ensure the right keys are used, but for now I didn't want to change too much here. Jira-Issue: MAV-7288
This converts the remaining ActiveJob jobs to be Sidekiq-only. These jobs all accept arguments, meaning we need to be careful about the serialisation and deserialisation of the arguments. To ensure the migration doesn't result in any missed jobs, this commit adds Sidekiq-only versions of all the jobs which are queued instead of the ActiveJob ones. Any ActiveJob jobs left in the queue will continue to execute. Jira-Issue: MAV-7288
thomasleese
force-pushed
the
use-sidekiq-directly-1
branch
from
52db5ba to
f377964
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an under the hood refactor that changes how we queue background jobs to use the Sidekiq API directly, rather than going via Active Job.
We're currently doing a mix of both, and we should standardise on a single approach. We have to use the Sidekiq API for rate limiting, unique jobs and bulk enqueuing, therefore we should standardise on using it everywhere.
To ensure no jobs are lost while this is deployed, we need to split this change in to three parts. This first part:
Depends on #6738
Jira Issue - MAV-7288