9423060299
* Fan out Feeds::ImportArticlesWorker Doing all that work within a single Sidekiq job has begun taking over an hour on DEV. Regardless of the reasons we did it that way originally, we should be able to handle this concurrently. If we cannot, we need to investigate why and handle it properly rather than consigning it to sequential work. * Fix specs The specs make assumptions about how the code under test is implemented. This commit does not change that, as much as I would like to. Instead, it just aligns the assumptions with the new implementation. The previous tests didn't actually represent reality though, since we can't get a Time or ActiveSupport::TimeWithZone instance inside of the `perform` method while running it through Sidekiq. Instead, the specs seem to be relying on the fact that the time instance gets serialized to ISO-8601/RFC3339 format and that that format is understood by Postgres. Otherwise, I'm not sure how it would work in production as written. The new specs reflect reality more closely. The `earlier_than` value will be converted into an ISO8601/RFC3339 string when passed through Sidekiq. * Add parens to perform_bulk call Turns out, we actually do this pretty consistently. I could've sworn I saw a bunch of these calls without parens. ¯\_(ツ)_/¯ * Improve variable naming This is not a list of ids, it's a list of lists of arguments for Sidekiq jobs, the inner of which contains an id, but that's not the only thing it contains.
45 lines
1.3 KiB
Ruby
45 lines
1.3 KiB
Ruby
module Feeds
|
|
class ImportArticlesWorker
|
|
include Sidekiq::Worker
|
|
|
|
sidekiq_options queue: :medium_priority, retry: 10, lock: :until_and_while_executing
|
|
|
|
# NOTE: [@rhymes] we need to default earlier_than to `nil` because sidekiq-cron,
|
|
# by using YAML to define jobs arguments does not support datetimes evaluated
|
|
# at runtime
|
|
def perform(user_ids = [], earlier_than = nil)
|
|
users_scope = User
|
|
|
|
if user_ids.present?
|
|
users_scope = users_scope.where(id: user_ids)
|
|
# we assume that forcing a single import should not take into account
|
|
# the last time a feed was fetched at
|
|
earlier_than = nil
|
|
else
|
|
earlier_than ||= 4.hours.ago
|
|
end
|
|
|
|
# For some reason `ActiveSupport::TimeWithZone#is_a?(Time)` evaluates to
|
|
# `true` so this works with any sort of time object
|
|
if earlier_than.is_a?(Time)
|
|
earlier_than = earlier_than.iso8601
|
|
end
|
|
|
|
users_scope.select(:id).find_in_batches do |batch|
|
|
arg_lists = batch.map { |user| [user.id, earlier_than] }
|
|
|
|
ForUser.perform_bulk(arg_lists)
|
|
end
|
|
end
|
|
|
|
class ForUser
|
|
include Sidekiq::Worker
|
|
|
|
def perform(user_ids, earlier_than)
|
|
users_scope = User.where(id: user_ids)
|
|
|
|
::Feeds::Import.call(users_scope: users_scope, earlier_than: earlier_than)
|
|
end
|
|
end
|
|
end
|
|
end
|