From 6d4d4a353fca51f5dcdc2e97928e89e6dabf4c37 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:54:05 -0400 Subject: [PATCH 01/21] wip --- Gemfile | 1 + Gemfile.lock | 2 + app/models/good_job/execution.rb | 9 +++- demo/app/jobs/example_job.rb | 1 + demo/app/jobs/other_job.rb | 3 ++ .../12_create_good_job_deferred_column.rb.erb | 21 ++++++++ .../active_job_extensions/paused_options.rb | 51 +++++++++++++++++++ lib/good_job/adapter.rb | 6 +-- 8 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb create mode 100644 lib/good_job/active_job_extensions/paused_options.rb diff --git a/Gemfile b/Gemfile index 3495fe34a..84e5010e0 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,7 @@ gem 'nokogiri' gem 'pg', platforms: [:mri, :mingw, :x64_mingw] gem 'rack', '~> 2.2' gem 'rails' +gem "byebug" platforms :ruby do gem "bootsnap" diff --git a/Gemfile.lock b/Gemfile.lock index 5bf2b6282..26c558136 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -135,6 +135,7 @@ GEM bootsnap (1.18.3) msgpack (~> 1.2) builder (3.2.4) + byebug (11.1.3) capybara (3.40.0) addressable matrix @@ -498,6 +499,7 @@ DEPENDENCIES appraisal benchmark-ips bootsnap + byebug capybara debug dotenv-rails diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 2b4c4850b..5d5a01289 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -91,11 +91,11 @@ def self.queue_parser(string) scope :unfinished, -> { where(finished_at: nil) } # Get executions that are not scheduled for a later time than now (i.e. jobs that - # are not scheduled or scheduled for earlier than the current time). + # are scheduled for earlier than the current time). Paused jobs are excluded. # @!method only_scheduled # @!scope class # @return [ActiveRecord::Relation] - scope :only_scheduled, -> { where(arel_table['scheduled_at'].lteq(bind_value('scheduled_at', DateTime.current, ActiveRecord::Type::DateTime))).or(where(scheduled_at: nil)) } + scope :only_scheduled, -> { where(arel_table['scheduled_at'].lteq(bind_value('scheduled_at', DateTime.current, ActiveRecord::Type::DateTime))) } # Order executions by priority (highest priority first). # @!method priority_ordered @@ -312,6 +312,8 @@ def self.next_scheduled_at(after: nil, limit: 100, now_limit: nil) # @return [Execution] # The new {Execution} instance representing the queued ActiveJob job. def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false) + puts 'Execution.enqueue' + # byebug ActiveSupport::Notifications.instrument("enqueue_job.good_job", { active_job: active_job, scheduled_at: scheduled_at, create_with_advisory_lock: create_with_advisory_lock }) do |instrument_payload| current_execution = CurrentThread.execution @@ -342,6 +344,9 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false end instrument_payload[:execution] = execution + if active_job.respond_to?(:good_job_paused) && active_job.good_job_paused + execution.scheduled_at = nil + end execution.save! if retried diff --git a/demo/app/jobs/example_job.rb b/demo/app/jobs/example_job.rb index 491eff8bf..842167ce1 100644 --- a/demo/app/jobs/example_job.rb +++ b/demo/app/jobs/example_job.rb @@ -31,6 +31,7 @@ def perform(batch, params) end def perform(type = SUCCESS_TYPE) + return true if type == SUCCESS_TYPE true elsif type == ERROR_ONCE_TYPE diff --git a/demo/app/jobs/other_job.rb b/demo/app/jobs/other_job.rb index 24f511a75..257a2017f 100644 --- a/demo/app/jobs/other_job.rb +++ b/demo/app/jobs/other_job.rb @@ -1,4 +1,7 @@ +require_relative '../../../lib/good_job/active_job_extensions/paused_options' + class OtherJob < ApplicationJob + include GoodJob::ActiveJobExtensions::PausedOptions JobError = Class.new(StandardError) def perform(*) diff --git a/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb b/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb new file mode 100644 index 000000000..06ae79fcf --- /dev/null +++ b/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class CreateIndexGoodJobJobsForCandidateLookup < ActiveRecord::Migration<%= migration_version %> + disable_ddl_transaction! + + # TODO: need to add the column to batches, jobs, and executions + + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_jobs, :deferred) + end + end +<%# TODO: probably need to add an index for this + add_index :good_jobs, [:priority, :created_at], order: { priority: "ASC NULLS LAST", created_at: :asc }, + where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup, + algorithm: :concurrently %> + end +end diff --git a/lib/good_job/active_job_extensions/paused_options.rb b/lib/good_job/active_job_extensions/paused_options.rb new file mode 100644 index 000000000..6abf02fcb --- /dev/null +++ b/lib/good_job/active_job_extensions/paused_options.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module GoodJob + module ActiveJobExtensions + # Allows configuring whether the job should start 'paused' when a job is enqueued. + # Configuration will apply either globally to the Job Class, or individually to jobs + # on initial enqueue and subsequent retries. + # + # @example + # # Include the concern to your job class: + # class MyJob < ApplicationJob + # include GoodJob::ActiveJobExtensions::PausedOptions + # self.good_job_paused = true + # end + # + # # Or, configure jobs individually to not notify: + # MyJob.set(good_job_paused: true).perform_later + + # TODO: should this be enabled globally/by default? (the 'do not run if nil' logic will always be active) + # TODO: consider renaming to 'Pauseable' + + module PausedOptions + extend ActiveSupport::Concern + + module Prepends + def enqueue(options = {}) + self.good_job_paused = options[:good_job_paused] if options.key?(:good_job_paused) + super + end + + def serialize + super.tap do |job_data| + # Only serialize the value if present to reduce the size of the serialized job + job_data["good_job_paused"] = good_job_paused unless good_job_paused.nil? + end + end + + def deserialize(job_data) + super + self.good_job_paused = job_data["good_job_paused"] + end + end + + included do + prepend Prepends + class_attribute :good_job_paused, instance_accessor: false, instance_predicate: false, default: nil + attr_accessor :good_job_paused + end + end + end +end diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 9029ee299..3a4e44941 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -92,7 +92,7 @@ def enqueue_all(active_jobs) executions = executions.select(&:persisted?) # prune unpersisted executions if execute_inline? - inline_executions = executions.select { |execution| (execution.scheduled_at.nil? || execution.scheduled_at <= Time.current) } + inline_executions = executions.select { |execution| execution.scheduled_at.present? && execution.scheduled_at <= Time.current } inline_executions.each(&:advisory_lock!) end end @@ -153,7 +153,7 @@ def enqueue_at(active_job, timestamp) return if GoodJob::Bulk.capture(active_job, queue_adapter: self) Rails.application.executor.wrap do - will_execute_inline = execute_inline? && (scheduled_at.nil? || scheduled_at <= Time.current) + will_execute_inline = execute_inline? && (scheduled_at.present? && scheduled_at <= Time.current) will_retry_inline = will_execute_inline && CurrentThread.execution&.active_job_id == active_job.job_id && !CurrentThread.retry_now if will_retry_inline @@ -171,7 +171,7 @@ def enqueue_at(active_job, timestamp) result = execution.perform retried_execution = result.retried - while retried_execution && (retried_execution.scheduled_at.nil? || retried_execution.scheduled_at <= Time.current) + while retried_execution && (retried_execution.scheduled_at.present? && retried_execution.scheduled_at <= Time.current) execution = retried_execution result = execution.perform retried_execution = result.retried From 9d9c7843502ec1bfb6c17acb51bce4ae636f5070 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:00:54 -0400 Subject: [PATCH 02/21] jobs queued within a paused batch are automatically paused --- app/models/good_job/batch.rb | 37 ++++++++++++++++++- demo/app/jobs/other_job.rb | 1 + .../active_job_extensions/paused_options.rb | 22 ++++++----- lib/good_job/adapter.rb | 1 + lib/good_job/bulk.rb | 6 ++- 5 files changed, 55 insertions(+), 12 deletions(-) diff --git a/app/models/good_job/batch.rb b/app/models/good_job/batch.rb index 17a635c45..cbdc261c1 100644 --- a/app/models/good_job/batch.rb +++ b/app/models/good_job/batch.rb @@ -119,7 +119,7 @@ def enqueue(active_jobs = [], **properties, &block) def add(active_jobs = nil, &block) record.save if record.new_record? - buffer = Bulk::Buffer.new + buffer = Bulk::Buffer.new(pause_jobs: properties[:paused] || false) buffer.add(active_jobs) buffer.capture(&block) if block @@ -130,6 +130,41 @@ def add(active_jobs = nil, &block) buffer.active_jobs end + def paused? + # TODO: consider querying to see if any jobs within the batch are paused, and if/how that should be represented if that result does not match properties[:paused] + properties[:paused] || false + end + + # TODO + # def pause; end + + def unpause + # TODO: consider raising an exception if the batch isn't paused in the first place + + # TODO: consider setting this at the end of the method, or doing something similar to help handle situations where an exception is raised during unpausing + assign_properties(paused: false) + + # TODO: this could be implemented with COALESCE and `jsonb_path_query(serialized_params, '$.scheduled_at.datetime()')` to extract the previously scheduled time within a single UPDATE, but that method is not available in PG12 (still supported at the time of writing) + unpaused_count = 0 + + while true + jobs = GoodJob::Job.where(batch_id: self.id, scheduled_at: nil).limit(1_000) + break if jobs.empty? + + jobs.each do |job| + job.update!(scheduled_at: job.serialized_params['scheduled_at'] || Time.current) + end + + jobs.collect(&:queue_name).tally.each do |q, c| + GoodJob::Notifier.notify({ queue_name: q, count: c }) + end + + unpaused_count += jobs.size + end + + unpaused_count + end + def active_jobs record.jobs.map(&:active_job) end diff --git a/demo/app/jobs/other_job.rb b/demo/app/jobs/other_job.rb index 257a2017f..bccb8f6a8 100644 --- a/demo/app/jobs/other_job.rb +++ b/demo/app/jobs/other_job.rb @@ -5,5 +5,6 @@ class OtherJob < ApplicationJob JobError = Class.new(StandardError) def perform(*) + # raise 'nope' end end diff --git a/lib/good_job/active_job_extensions/paused_options.rb b/lib/good_job/active_job_extensions/paused_options.rb index 6abf02fcb..85534718a 100644 --- a/lib/good_job/active_job_extensions/paused_options.rb +++ b/lib/good_job/active_job_extensions/paused_options.rb @@ -17,6 +17,8 @@ module ActiveJobExtensions # MyJob.set(good_job_paused: true).perform_later # TODO: should this be enabled globally/by default? (the 'do not run if nil' logic will always be active) + # Yeah I think this is going to need to be, otherwise you can add jobs to a paused batch and they won't actually be paused + # TODO: consider renaming to 'Pauseable' module PausedOptions @@ -28,17 +30,17 @@ def enqueue(options = {}) super end - def serialize - super.tap do |job_data| - # Only serialize the value if present to reduce the size of the serialized job - job_data["good_job_paused"] = good_job_paused unless good_job_paused.nil? - end - end + # good_job_paused is intentionally excluded from the serialized params so we fully rely on the scheduled_at value once the job is enqueued + # def serialize + # super.tap do |job_data| + # # job_data["good_job_paused"] = good_job_paused unless good_job_paused.nil? + # end + # end - def deserialize(job_data) - super - self.good_job_paused = job_data["good_job_paused"] - end + # def deserialize(job_data) + # super + # self.good_job_paused = job_data["good_job_paused"] + # end end included do diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 3a4e44941..e5de4796a 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -62,6 +62,7 @@ def enqueue_all(active_jobs) if GoodJob::Execution.discrete_support? execution.make_discrete execution.scheduled_at = current_time if execution.scheduled_at == execution.created_at + execution.scheduled_at = nil if active_job.respond_to?(:good_job_paused) && active_job.good_job_paused end execution.created_at = current_time diff --git a/lib/good_job/bulk.rb b/lib/good_job/bulk.rb index 9511ef30c..85b77b4e1 100644 --- a/lib/good_job/bulk.rb +++ b/lib/good_job/bulk.rb @@ -60,8 +60,9 @@ def self.unbuffer end class Buffer - def initialize + def initialize(pause_jobs: false) @values = [] + @pause_jobs = pause_jobs end def capture @@ -79,6 +80,9 @@ def add(active_jobs, queue_adapter: nil) adapter = queue_adapter || active_job.class.queue_adapter raise Error, "Jobs must have a Queue Adapter" unless adapter + # TODO: should explicitly setting `good_job_paused = false` on a job override this? + active_job.good_job_paused = true if @pause_jobs && active_job.respond_to?(:good_job_paused) + [active_job, adapter] end @values.append(*new_pairs) From 508e55699c3b51cd2c24a10214b5162967f408fb Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:10:57 -0400 Subject: [PATCH 03/21] rm irrelevant changes --- app/models/good_job/execution.rb | 2 -- .../12_create_good_job_deferred_column.rb.erb | 21 ------------------- 2 files changed, 23 deletions(-) delete mode 100644 lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 5d5a01289..525e47d73 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -312,8 +312,6 @@ def self.next_scheduled_at(after: nil, limit: 100, now_limit: nil) # @return [Execution] # The new {Execution} instance representing the queued ActiveJob job. def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false) - puts 'Execution.enqueue' - # byebug ActiveSupport::Notifications.instrument("enqueue_job.good_job", { active_job: active_job, scheduled_at: scheduled_at, create_with_advisory_lock: create_with_advisory_lock }) do |instrument_payload| current_execution = CurrentThread.execution diff --git a/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb b/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb deleted file mode 100644 index 06ae79fcf..000000000 --- a/lib/generators/good_job/templates/update/migrations/12_create_good_job_deferred_column.rb.erb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -class CreateIndexGoodJobJobsForCandidateLookup < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - # TODO: need to add the column to batches, jobs, and executions - - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_jobs, :deferred) - end - end -<%# TODO: probably need to add an index for this - add_index :good_jobs, [:priority, :created_at], order: { priority: "ASC NULLS LAST", created_at: :asc }, - where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup, - algorithm: :concurrently %> - end -end From ec11c448a7c11c9cd60d115d3e61978de5b3206b Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:14:45 -0400 Subject: [PATCH 04/21] appease linter --- app/models/good_job/batch.rb | 4 ++-- app/models/good_job/execution.rb | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/good_job/batch.rb b/app/models/good_job/batch.rb index cbdc261c1..2c7e51f7a 100644 --- a/app/models/good_job/batch.rb +++ b/app/models/good_job/batch.rb @@ -147,8 +147,8 @@ def unpause # TODO: this could be implemented with COALESCE and `jsonb_path_query(serialized_params, '$.scheduled_at.datetime()')` to extract the previously scheduled time within a single UPDATE, but that method is not available in PG12 (still supported at the time of writing) unpaused_count = 0 - while true - jobs = GoodJob::Job.where(batch_id: self.id, scheduled_at: nil).limit(1_000) + loop do + jobs = GoodJob::Job.where(batch_id: id, scheduled_at: nil).limit(1_000) break if jobs.empty? jobs.each do |job| diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 525e47d73..2c60c72f5 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -342,9 +342,7 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false end instrument_payload[:execution] = execution - if active_job.respond_to?(:good_job_paused) && active_job.good_job_paused - execution.scheduled_at = nil - end + execution.scheduled_at = nil if active_job.respond_to?(:good_job_paused) && active_job.good_job_paused execution.save! if retried From e368b3c77a48c3e957490014f485c3c66fd496d6 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:15:03 -0400 Subject: [PATCH 05/21] remove byebug --- Gemfile | 1 - Gemfile.lock | 2 -- 2 files changed, 3 deletions(-) diff --git a/Gemfile b/Gemfile index 84e5010e0..3495fe34a 100644 --- a/Gemfile +++ b/Gemfile @@ -26,7 +26,6 @@ gem 'nokogiri' gem 'pg', platforms: [:mri, :mingw, :x64_mingw] gem 'rack', '~> 2.2' gem 'rails' -gem "byebug" platforms :ruby do gem "bootsnap" diff --git a/Gemfile.lock b/Gemfile.lock index 26c558136..5bf2b6282 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -135,7 +135,6 @@ GEM bootsnap (1.18.3) msgpack (~> 1.2) builder (3.2.4) - byebug (11.1.3) capybara (3.40.0) addressable matrix @@ -499,7 +498,6 @@ DEPENDENCIES appraisal benchmark-ips bootsnap - byebug capybara debug dotenv-rails From 1803f8b7b5c39f3dbbd04ce9804d022cbad3f7fe Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:21:16 -0400 Subject: [PATCH 06/21] revert dev change that was breaking tests --- demo/app/jobs/example_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/app/jobs/example_job.rb b/demo/app/jobs/example_job.rb index 842167ce1..bed3c8ef2 100644 --- a/demo/app/jobs/example_job.rb +++ b/demo/app/jobs/example_job.rb @@ -31,7 +31,7 @@ def perform(batch, params) end def perform(type = SUCCESS_TYPE) - return true + # return true if type == SUCCESS_TYPE true elsif type == ERROR_ONCE_TYPE From f949fd0e92d93967fd0b7aae36601e4fa1c0321f Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:31:53 -0400 Subject: [PATCH 07/21] reworked pause extension to be included globally --- demo/app/jobs/other_job.rb | 3 --- lib/good_job.rb | 1 + .../{paused_options.rb => pauseable.rb} | 14 ++++++-------- 3 files changed, 7 insertions(+), 11 deletions(-) rename lib/good_job/active_job_extensions/{paused_options.rb => pauseable.rb} (77%) diff --git a/demo/app/jobs/other_job.rb b/demo/app/jobs/other_job.rb index bccb8f6a8..2fabd5aa0 100644 --- a/demo/app/jobs/other_job.rb +++ b/demo/app/jobs/other_job.rb @@ -1,7 +1,4 @@ -require_relative '../../../lib/good_job/active_job_extensions/paused_options' - class OtherJob < ApplicationJob - include GoodJob::ActiveJobExtensions::PausedOptions JobError = Class.new(StandardError) def perform(*) diff --git a/lib/good_job.rb b/lib/good_job.rb index 8208f5ad7..de37ad2fa 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -14,6 +14,7 @@ require_relative "good_job/active_job_extensions/interrupt_errors" require_relative "good_job/active_job_extensions/labels" require_relative "good_job/active_job_extensions/notify_options" +require_relative "good_job/active_job_extensions/pauseable" require_relative "good_job/overridable_connection" require_relative "good_job/bulk" diff --git a/lib/good_job/active_job_extensions/paused_options.rb b/lib/good_job/active_job_extensions/pauseable.rb similarity index 77% rename from lib/good_job/active_job_extensions/paused_options.rb rename to lib/good_job/active_job_extensions/pauseable.rb index 85534718a..0ca5eecfb 100644 --- a/lib/good_job/active_job_extensions/paused_options.rb +++ b/lib/good_job/active_job_extensions/pauseable.rb @@ -7,21 +7,16 @@ module ActiveJobExtensions # on initial enqueue and subsequent retries. # # @example - # # Include the concern to your job class: # class MyJob < ApplicationJob - # include GoodJob::ActiveJobExtensions::PausedOptions # self.good_job_paused = true # end # # # Or, configure jobs individually to not notify: # MyJob.set(good_job_paused: true).perform_later + # + # See also - GoodJob:Batch#new's `paused` option - # TODO: should this be enabled globally/by default? (the 'do not run if nil' logic will always be active) - # Yeah I think this is going to need to be, otherwise you can add jobs to a paused batch and they won't actually be paused - - # TODO: consider renaming to 'Pauseable' - - module PausedOptions + module Pauseable extend ActiveSupport::Concern module Prepends @@ -51,3 +46,6 @@ def enqueue(options = {}) end end end + +# Jobs can be paused through batches which rely on good_job_paused being available, so this must be included globally +ActiveJob::Base.include GoodJob::ActiveJobExtensions::Pauseable From 4998922bfd148da0c4c7cdadc2da719ebd3c8a64 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:37:02 -0400 Subject: [PATCH 08/21] comment --- lib/good_job/active_job_extensions/pauseable.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/good_job/active_job_extensions/pauseable.rb b/lib/good_job/active_job_extensions/pauseable.rb index 0ca5eecfb..8acc21687 100644 --- a/lib/good_job/active_job_extensions/pauseable.rb +++ b/lib/good_job/active_job_extensions/pauseable.rb @@ -26,6 +26,7 @@ def enqueue(options = {}) end # good_job_paused is intentionally excluded from the serialized params so we fully rely on the scheduled_at value once the job is enqueued + # TODO: remove before merge # def serialize # super.tap do |job_data| # # job_data["good_job_paused"] = good_job_paused unless good_job_paused.nil? From bf33a9b4b0d05b84472ecc1af45df742bb3cca6a Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:47:22 -0400 Subject: [PATCH 09/21] todo --- README.md | 8 +++++++- app/models/good_job/batch.rb | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 045423075..bac603178 100644 --- a/README.md +++ b/README.md @@ -722,7 +722,7 @@ Batches track a set of jobs, and enqueue an optional callback job when all of th - [`GoodJob::Batch`](app/models/good_job/batch.rb) has a number of assignable attributes and methods: ```ruby -batch = GoodJob::Batch.new +batch = GoodJob::Batch.new # or .new(paused: true) to pause all jobs added to the batch batch.description = "My batch" batch.on_finish = "MyBatchCallbackJob" # Callback job when all jobs have finished batch.on_success = "MyBatchCallbackJob" # Callback job when/if all jobs have succeeded @@ -734,12 +734,14 @@ batch.add do MyJob.perform_later end batch.enqueue +batch.unpause # Unpauses all jobs within the batch, allowing them to be executed batch.discarded? # => Boolean batch.discarded_at # => batch.finished? # => Boolean batch.finished_at # => batch.succeeded? # => Boolean +batch.paused? # => Boolean // TODO: expand on what this method does batch.active_jobs # => Array of ActiveJob::Base-inherited jobs that are part of the batch batch = GoodJob::Batch.find(batch.id) @@ -831,6 +833,10 @@ end GoodJob::Batch.enqueue(on_finish: BatchJob) ``` +#### Pausing batches + +// TODO: document how to pause/unpause a batch (potentially create as an entirely separate section about pausing things?) + #### Other batch details - Whether to enqueue a callback job is evaluated once the batch is in an `enqueued?`-state by using `GoodJob::Batch.enqueue` or `batch.enqueue`. diff --git a/app/models/good_job/batch.rb b/app/models/good_job/batch.rb index 2c7e51f7a..3f5b192dd 100644 --- a/app/models/good_job/batch.rb +++ b/app/models/good_job/batch.rb @@ -130,14 +130,17 @@ def add(active_jobs = nil, &block) buffer.active_jobs end + # TODO: document def paused? # TODO: consider querying to see if any jobs within the batch are paused, and if/how that should be represented if that result does not match properties[:paused] + # I think there are probably going to need to be separate methods for "is the batch set to pause all jobs within it" and "does the batch contain any paused jobs" as those cases aren't always lined up properties[:paused] || false end # TODO # def pause; end + # TODO: document def unpause # TODO: consider raising an exception if the batch isn't paused in the first place From 10e14048eb8246114f017099f87cc02d9abcca82 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:55:31 -0400 Subject: [PATCH 10/21] linter - use load hook --- lib/good_job/active_job_extensions/pauseable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/good_job/active_job_extensions/pauseable.rb b/lib/good_job/active_job_extensions/pauseable.rb index 8acc21687..86d98c34b 100644 --- a/lib/good_job/active_job_extensions/pauseable.rb +++ b/lib/good_job/active_job_extensions/pauseable.rb @@ -49,4 +49,4 @@ def enqueue(options = {}) end # Jobs can be paused through batches which rely on good_job_paused being available, so this must be included globally -ActiveJob::Base.include GoodJob::ActiveJobExtensions::Pauseable +ActiveSupport.on_load(:active_job) { include GoodJob::ActiveJobExtensions::Pauseable } From cceee54dd2f8322550d854f95056ca1b1c5ca585 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Tue, 16 Apr 2024 10:04:52 -0400 Subject: [PATCH 11/21] added a stopgap and TODO for an edge case --- app/models/good_job/batch.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/good_job/batch.rb b/app/models/good_job/batch.rb index 3f5b192dd..442c8fca1 100644 --- a/app/models/good_job/batch.rb +++ b/app/models/good_job/batch.rb @@ -144,6 +144,9 @@ def paused? def unpause # TODO: consider raising an exception if the batch isn't paused in the first place + # TODO: encountered this edge case with unpausing a non-persisted batch + raise 'need to investigate how jobs are handled when batches arent persisted or else the query will end up being where batch is null' if id.nil? + # TODO: consider setting this at the end of the method, or doing something similar to help handle situations where an exception is raised during unpausing assign_properties(paused: false) From f66c5d79bad4b6de6deccc7919b06a0dde1d81f9 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Tue, 16 Apr 2024 13:43:10 -0400 Subject: [PATCH 12/21] fixed a issue that would cause the jobs-to-unpause query to break and select all paused jobs if the batch had not been persisted --- app/models/good_job/batch.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/models/good_job/batch.rb b/app/models/good_job/batch.rb index 442c8fca1..ee2e32270 100644 --- a/app/models/good_job/batch.rb +++ b/app/models/good_job/batch.rb @@ -144,9 +144,6 @@ def paused? def unpause # TODO: consider raising an exception if the batch isn't paused in the first place - # TODO: encountered this edge case with unpausing a non-persisted batch - raise 'need to investigate how jobs are handled when batches arent persisted or else the query will end up being where batch is null' if id.nil? - # TODO: consider setting this at the end of the method, or doing something similar to help handle situations where an exception is raised during unpausing assign_properties(paused: false) @@ -154,7 +151,7 @@ def unpause unpaused_count = 0 loop do - jobs = GoodJob::Job.where(batch_id: id, scheduled_at: nil).limit(1_000) + jobs = record.jobs.where(scheduled_at: nil).limit(1_000) break if jobs.empty? jobs.each do |job| From 84527cbe29e07c29e3783b3c941bba4b430e6e9d Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Wed, 17 Apr 2024 10:34:30 -0400 Subject: [PATCH 13/21] working on tests --- Gemfile | 2 ++ Gemfile.lock | 4 ++++ demo/config/database.yml | 1 + spec/app/filters/good_job/jobs_filter_spec.rb | 7 +++++++ todo.txt | 2 ++ 5 files changed, 16 insertions(+) create mode 100644 todo.txt diff --git a/Gemfile b/Gemfile index 3495fe34a..41a871684 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,8 @@ gemspec gem 'activerecord-jdbcpostgresql-adapter', platforms: [:jruby] gem 'appraisal' +gem 'awesome_print' +gem 'byebug' gem 'matrix' gem 'nokogiri' gem 'pg', platforms: [:mri, :mingw, :x64_mingw] diff --git a/Gemfile.lock b/Gemfile.lock index 5bf2b6282..c42245170 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -121,6 +121,7 @@ GEM async async-pool (0.4.0) async (>= 1.25) + awesome_print (1.9.2) base64 (0.2.0) benchmark-ips (2.13.0) better_html (2.0.2) @@ -135,6 +136,7 @@ GEM bootsnap (1.18.3) msgpack (~> 1.2) builder (3.2.4) + byebug (11.1.3) capybara (3.40.0) addressable matrix @@ -496,8 +498,10 @@ DEPENDENCIES activerecord-explain-analyze activerecord-jdbcpostgresql-adapter appraisal + awesome_print benchmark-ips bootsnap + byebug capybara debug dotenv-rails diff --git a/demo/config/database.yml b/demo/config/database.yml index 3fba25c7e..a5db7b52c 100644 --- a/demo/config/database.yml +++ b/demo/config/database.yml @@ -59,6 +59,7 @@ development: test: <<: *default database: good_job_test + url: <%= ENV['DATABASE_URL'] %> reaping_frequency: 0 # As with config/credentials.yml, you never want to store sensitive information, diff --git a/spec/app/filters/good_job/jobs_filter_spec.rb b/spec/app/filters/good_job/jobs_filter_spec.rb index 83daa3c8f..2bdb0787f 100644 --- a/spec/app/filters/good_job/jobs_filter_spec.rb +++ b/spec/app/filters/good_job/jobs_filter_spec.rb @@ -11,15 +11,21 @@ allow(GoodJob).to receive_messages(retry_on_unhandled_error: false, preserve_job_records: true) ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :external) + byebug ExampleJob.set(queue: 'cron').perform_later + byebug + GoodJob::Job.order(created_at: :asc).last.update!(cron_key: "frequent_cron") ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) ExampleJob.set(queue: 'default').perform_later(ExampleJob::SUCCESS_TYPE) ExampleJob.set(queue: 'mice').perform_later(ExampleJob::ERROR_ONCE_TYPE) + byebug Timecop.travel 1.hour.ago ExampleJob.set(queue: 'elephants').perform_later(ExampleJob::DEAD_TYPE) + byebug + 5.times do Timecop.travel 5.minutes GoodJob.perform_inline @@ -27,6 +33,7 @@ Timecop.return running_job = ExampleJob.perform_later('success') + byebug running_execution = GoodJob::Execution.find(running_job.provider_job_id) running_execution.update!( finished_at: nil diff --git a/todo.txt b/todo.txt new file mode 100644 index 000000000..1a28936e0 --- /dev/null +++ b/todo.txt @@ -0,0 +1,2 @@ +* Indicate paused status in the jobs dashboard + From 749b46434dfcf265bd7cfc03d2e7b422a3e963a4 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:35:54 -0400 Subject: [PATCH 14/21] cleanup --- spec/app/filters/good_job/jobs_filter_spec.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/app/filters/good_job/jobs_filter_spec.rb b/spec/app/filters/good_job/jobs_filter_spec.rb index 2bdb0787f..976a59d62 100644 --- a/spec/app/filters/good_job/jobs_filter_spec.rb +++ b/spec/app/filters/good_job/jobs_filter_spec.rb @@ -11,20 +11,16 @@ allow(GoodJob).to receive_messages(retry_on_unhandled_error: false, preserve_job_records: true) ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :external) - byebug ExampleJob.set(queue: 'cron').perform_later - byebug GoodJob::Job.order(created_at: :asc).last.update!(cron_key: "frequent_cron") ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) ExampleJob.set(queue: 'default').perform_later(ExampleJob::SUCCESS_TYPE) ExampleJob.set(queue: 'mice').perform_later(ExampleJob::ERROR_ONCE_TYPE) - byebug Timecop.travel 1.hour.ago ExampleJob.set(queue: 'elephants').perform_later(ExampleJob::DEAD_TYPE) - byebug 5.times do Timecop.travel 5.minutes @@ -33,7 +29,6 @@ Timecop.return running_job = ExampleJob.perform_later('success') - byebug running_execution = GoodJob::Execution.find(running_job.provider_job_id) running_execution.update!( finished_at: nil From bce4dafe53165c5d9e05ec64746810b4ce83fef9 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Fri, 19 Apr 2024 14:40:45 -0400 Subject: [PATCH 15/21] fixed test failures caused by scheduled_at being nil, need to confirm the intended behavior is that it is always set --- app/models/good_job/execution.rb | 3 ++- lib/good_job/adapter.rb | 3 ++- spec/app/models/good_job/execution_spec.rb | 8 ++++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 2c60c72f5..a513170e9 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -329,7 +329,8 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) end else - execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) + # Based on the discussion on github, I think the intent in current GJ versions is that scheduled_at is always set - need to confirm + execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at || Time.current }) execution.make_discrete if discrete_support? end diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index e5de4796a..11410f216 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -147,7 +147,8 @@ def enqueue_all(active_jobs) # @param timestamp [Integer, nil] the epoch time to perform the job # @return [GoodJob::Execution] def enqueue_at(active_job, timestamp) - scheduled_at = timestamp ? Time.zone.at(timestamp) : nil + # Based on the discussion on github, I think the intent in current GJ versions is that scheduled_at is always set - need to confirm + scheduled_at = timestamp ? Time.zone.at(timestamp) : Time.current # nil # If there is a currently open Bulk in the current thread, direct the # job there to be enqueued using enqueue_all diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb index 369683120..d6f744479 100644 --- a/spec/app/models/good_job/execution_spec.rb +++ b/spec/app/models/good_job/execution_spec.rb @@ -181,8 +181,10 @@ def perform(result_value = nil, raise_error: false) end context 'with multiple jobs' do + let!(:sched) { Time.current } + def job_params - { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" } } + { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" }, scheduled_at: sched } end let!(:older_job) { described_class.create!(job_params.merge(created_at: 10.minutes.ago)) } @@ -204,8 +206,10 @@ def job_params end context "with multiple jobs and ordered queues" do + let!(:sched) { Time.current } + def job_params - { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" } } + { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" }, scheduled_at: sched } end let(:parsed_queues) { { include: %w{one two}, ordered_queues: true } } From 18b58ffe05ad649cd1573f4d2fb665aee3f4c7a6 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Fri, 19 Apr 2024 14:42:04 -0400 Subject: [PATCH 16/21] reverted local-only changes --- Gemfile | 2 -- Gemfile.lock | 4 ---- demo/config/database.yml | 1 - 3 files changed, 7 deletions(-) diff --git a/Gemfile b/Gemfile index 41a871684..3495fe34a 100644 --- a/Gemfile +++ b/Gemfile @@ -21,8 +21,6 @@ gemspec gem 'activerecord-jdbcpostgresql-adapter', platforms: [:jruby] gem 'appraisal' -gem 'awesome_print' -gem 'byebug' gem 'matrix' gem 'nokogiri' gem 'pg', platforms: [:mri, :mingw, :x64_mingw] diff --git a/Gemfile.lock b/Gemfile.lock index c42245170..5bf2b6282 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -121,7 +121,6 @@ GEM async async-pool (0.4.0) async (>= 1.25) - awesome_print (1.9.2) base64 (0.2.0) benchmark-ips (2.13.0) better_html (2.0.2) @@ -136,7 +135,6 @@ GEM bootsnap (1.18.3) msgpack (~> 1.2) builder (3.2.4) - byebug (11.1.3) capybara (3.40.0) addressable matrix @@ -498,10 +496,8 @@ DEPENDENCIES activerecord-explain-analyze activerecord-jdbcpostgresql-adapter appraisal - awesome_print benchmark-ips bootsnap - byebug capybara debug dotenv-rails diff --git a/demo/config/database.yml b/demo/config/database.yml index a5db7b52c..3fba25c7e 100644 --- a/demo/config/database.yml +++ b/demo/config/database.yml @@ -59,7 +59,6 @@ development: test: <<: *default database: good_job_test - url: <%= ENV['DATABASE_URL'] %> reaping_frequency: 0 # As with config/credentials.yml, you never want to store sensitive information, From 768ab276a0f5e6790aa2ac2e712787e4c5fc5a30 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 22 Apr 2024 09:53:16 -0400 Subject: [PATCH 17/21] linter --- spec/app/models/good_job/execution_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb index d6f744479..b893ddaaf 100644 --- a/spec/app/models/good_job/execution_spec.rb +++ b/spec/app/models/good_job/execution_spec.rb @@ -181,12 +181,11 @@ def perform(result_value = nil, raise_error: false) end context 'with multiple jobs' do - let!(:sched) { Time.current } - def job_params { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" }, scheduled_at: sched } end + let!(:sched) { Time.current } let!(:older_job) { described_class.create!(job_params.merge(created_at: 10.minutes.ago)) } let!(:newer_job) { described_class.create!(job_params.merge(created_at: 5.minutes.ago)) } let!(:low_priority_job) { described_class.create!(job_params.merge(priority: 20)) } @@ -206,12 +205,11 @@ def job_params end context "with multiple jobs and ordered queues" do - let!(:sched) { Time.current } - def job_params { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" }, scheduled_at: sched } end + let!(:sched) { Time.current } let(:parsed_queues) { { include: %w{one two}, ordered_queues: true } } let!(:queue_two_job) { described_class.create!(job_params.merge(queue_name: "two", created_at: 10.minutes.ago, priority: 100)) } let!(:queue_one_job) { described_class.create!(job_params.merge(queue_name: "one", created_at: 1.minute.ago, priority: 1)) } From a31084e524ff9cd0be097f611901f148113fa3e3 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:09:46 -0400 Subject: [PATCH 18/21] repaired tests --- app/models/good_job/execution.rb | 3 ++- spec/app/models/good_job/execution_spec.rb | 6 +++--- spec/lib/good_job/adapter_spec.rb | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index a513170e9..79031b628 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -330,8 +330,9 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false end else # Based on the discussion on github, I think the intent in current GJ versions is that scheduled_at is always set - need to confirm - execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at || Time.current }) + execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) execution.make_discrete if discrete_support? + execution.scheduled_at ||= Time.current # set after make_discrete so it can manage assigning both created_at and scheduled_at simultaneously end if create_with_advisory_lock diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb index b893ddaaf..fa033f9a9 100644 --- a/spec/app/models/good_job/execution_spec.rb +++ b/spec/app/models/good_job/execution_spec.rb @@ -63,7 +63,7 @@ def perform(result_value = nil, raise_error: false) context 'when NOT discrete' do before { allow(described_class).to receive(:discrete_support?).and_return(false) } - it 'does not assign id, scheduled_at' do + it 'does not assign id, does assign scheduled_at' do expect { described_class.enqueue(active_job) }.to change(described_class, :count).by(1) execution = described_class.last @@ -71,7 +71,7 @@ def perform(result_value = nil, raise_error: false) expect(execution).to have_attributes( is_discrete: nil, active_job_id: active_job.job_id, - scheduled_at: nil + scheduled_at: within(1).of(Time.current) ) end end @@ -88,7 +88,7 @@ def perform(result_value = nil, raise_error: false) serialized_params: a_kind_of(Hash), queue_name: 'test', priority: 50, - scheduled_at: nil + scheduled_at: within(1).of(Time.current) ) end diff --git a/spec/lib/good_job/adapter_spec.rb b/spec/lib/good_job/adapter_spec.rb index 54611d214..0c3d1d6ef 100644 --- a/spec/lib/good_job/adapter_spec.rb +++ b/spec/lib/good_job/adapter_spec.rb @@ -56,7 +56,7 @@ expect(GoodJob::Execution).to have_received(:enqueue).with( active_job, - scheduled_at: nil + scheduled_at: be_within(1).of(Time.current) ) end From e27b9f5cf42c8e18767fb6db0f07785080523ab4 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:16:42 -0400 Subject: [PATCH 19/21] cleanup --- app/models/good_job/execution.rb | 1 - lib/good_job/adapter.rb | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 79031b628..357550b7d 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -329,7 +329,6 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) end else - # Based on the discussion on github, I think the intent in current GJ versions is that scheduled_at is always set - need to confirm execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) execution.make_discrete if discrete_support? execution.scheduled_at ||= Time.current # set after make_discrete so it can manage assigning both created_at and scheduled_at simultaneously diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 11410f216..b774e84ba 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -147,8 +147,7 @@ def enqueue_all(active_jobs) # @param timestamp [Integer, nil] the epoch time to perform the job # @return [GoodJob::Execution] def enqueue_at(active_job, timestamp) - # Based on the discussion on github, I think the intent in current GJ versions is that scheduled_at is always set - need to confirm - scheduled_at = timestamp ? Time.zone.at(timestamp) : Time.current # nil + scheduled_at = timestamp ? Time.zone.at(timestamp) : Time.current # If there is a currently open Bulk in the current thread, direct the # job there to be enqueued using enqueue_all From 1f266dbbc7d6e21406c66e0f2db4f08eb1eda603 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:39:57 -0400 Subject: [PATCH 20/21] rm comment as tests seem to depend on line numbering --- demo/app/jobs/example_job.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/demo/app/jobs/example_job.rb b/demo/app/jobs/example_job.rb index bed3c8ef2..491eff8bf 100644 --- a/demo/app/jobs/example_job.rb +++ b/demo/app/jobs/example_job.rb @@ -31,7 +31,6 @@ def perform(batch, params) end def perform(type = SUCCESS_TYPE) - # return true if type == SUCCESS_TYPE true elsif type == ERROR_ONCE_TYPE From 7c599a78ceef6c2404835c22cad03764a19b84f8 Mon Sep 17 00:00:00 2001 From: pgvsalamander <105011449+pgvsalamander@users.noreply.github.com> Date: Mon, 22 Apr 2024 12:10:12 -0400 Subject: [PATCH 21/21] relaxed how close created_at and scheduled_at must be to pass a test --- spec/app/models/good_job/execution_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb index fa033f9a9..92d7519b8 100644 --- a/spec/app/models/good_job/execution_spec.rb +++ b/spec/app/models/good_job/execution_spec.rb @@ -700,7 +700,7 @@ def job_params job_class: good_job.job_class, queue_name: good_job.queue_name, created_at: within(0.001).of(good_job.performed_at), - scheduled_at: within(0.001).of(good_job.created_at), + scheduled_at: within(0.1).of(good_job.created_at), finished_at: within(1.second).of(Time.current), error: nil, serialized_params: good_job.serialized_params