Skip to content

Commit

Permalink
Remove a bit of code around discrete distinction
Browse files Browse the repository at this point in the history
  • Loading branch information
Earlopain committed Jul 8, 2024
1 parent c879d5d commit 5be5c28
Show file tree
Hide file tree
Showing 10 changed files with 3 additions and 141 deletions.
19 changes: 0 additions & 19 deletions app/models/good_job/base_execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,6 @@ def params_execution_count
def coalesce_scheduled_at_created_at
arel_table.coalesce(arel_table['scheduled_at'], arel_table['created_at'])
end

def discrete_support?
true
end
end

def discrete?
is_discrete?
end

# Build an ActiveJob instance and deserialize the arguments, using `#active_job_data`.
Expand Down Expand Up @@ -545,17 +537,6 @@ def executable?
false
end

def make_discrete
self.is_discrete = true
self.id = active_job_id
self.job_class = serialized_params['job_class']
self.executions_count ||= 0

current_time = Time.current
self.created_at ||= current_time
self.scheduled_at ||= current_time
end

# Return formatted serialized_params for display in the dashboard
# @return [Hash]
def display_serialized_params
Expand Down
2 changes: 0 additions & 2 deletions app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ class Job < BaseExecution
# Errored but will not be retried
scope :discarded, -> { finished.where.not(error: nil) }

scope :unfinished_undiscrete, -> { where(finished_at: nil, retried_good_job_id: nil, is_discrete: [nil, false]) }

# TODO: it would be nice to enforce these values at the model
# validates :active_job_id, presence: true
# validates :scheduled_at, presence: true
Expand Down
3 changes: 0 additions & 3 deletions demo/config/initializers/good_job.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
Rails.application.configure do
# TODO: Remove on GoodJob 4.0 release
config.good_job.smaller_number_is_higher_priority = true

config.good_job.cron = {
example: {
cron: '*/5 * * * * *', # every 5 seconds
Expand Down
4 changes: 1 addition & 3 deletions demo/db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
time_increments = (1.minute..10.minutes).to_a
job_classes = ['ExampleJob', 'OtherJob']
queue_names = ["default", "mice", "elephants"]
discrete_is_supported = GoodJob::BaseExecution.discrete_support?

jobs_data = []
loop do
Expand Down Expand Up @@ -36,8 +35,7 @@
scheduled_at: nil,
performed_at: nil,
finished_at: nil,
error: nil,
is_discrete: discrete_is_supported
error: nil
}

start_date += time_increments.sample
Expand Down
2 changes: 1 addition & 1 deletion lib/good_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def self.perform_inline(queue_string = "*", limit: nil)
# Tests whether GoodJob can be safely upgraded to v4
# @return [Boolean]
def self.v4_ready?
GoodJob::Job.discrete_support? && GoodJob::Job.where(finished_at: nil).where(is_discrete: [nil, false]).none?
true
end

# Deprecator for providing deprecation warnings.
Expand Down
38 changes: 0 additions & 38 deletions scripts/benchmark_discrete_jobs.rb

This file was deleted.

6 changes: 0 additions & 6 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,6 @@ def perform(*)
end

before do
allow(described_class).to receive(:discrete_support?).and_return(false)

stub_const "RUN_JOBS", Concurrent::Array.new
stub_const 'TestJob', (Class.new(ActiveJob::Base) do
self.queue_name = 'test'
Expand Down Expand Up @@ -951,8 +949,6 @@ def job_params
context 'when Discrete' do
before do
ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
allow(described_class).to receive(:discrete_support?).and_return(true)
good_job.update!(is_discrete: true)
end

it 'updates the Execution record and creates a DiscreteExecution record' do
Expand Down Expand Up @@ -983,10 +979,8 @@ def job_params
let!(:good_job) { described_class.enqueue(active_job) }

before do
allow(described_class).to receive(:discrete_support?).and_return(true)
allow(GoodJob).to receive(:preserve_job_records).and_return(true)
TestJob.retry_on(StandardError, wait: 1.hour, attempts: 2) { nil }
good_job.update!(is_discrete: true)
end

it 'updates the existing Execution/Job record instead of creating a new one' do
Expand Down
18 changes: 0 additions & 18 deletions spec/lib/good_job/active_job_extensions/concurrency_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,24 +248,6 @@ def perform(*)
expect(GoodJob::Job.first.concurrency_key).to be_present
expect(GoodJob::Job.first).not_to be_finished
end

context 'when not discrete' do
it 'preserves the key value across retries' do
TestJob.set(wait_until: 5.minutes.ago).perform_later(name: "Alice")
GoodJob::Job.first.update!(is_discrete: false)

original_concurrency_key = GoodJob::Job.last.concurrency_key
expect(original_concurrency_key).to be_present

begin
GoodJob.perform_inline
rescue StandardError
nil
end

expect(GoodJob::Job.last.concurrency_key).to eq original_concurrency_key
end
end
end
end

Expand Down
34 changes: 0 additions & 34 deletions spec/lib/good_job/adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@

describe '#enqueue' do
it 'sets default values' do
allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)

active_job = ExampleJob.new
adapter.enqueue(active_job)

Expand All @@ -37,16 +35,6 @@
priority: 0,
scheduled_at: be_within(1.second).of(Time.current)
)

allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
active_job = ExampleJob.new
adapter.enqueue(active_job)

expect(GoodJob::Job.last).to have_attributes(
queue_name: 'default',
priority: 0
# scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
)
end

it 'calls GoodJob::Job.enqueue with parameters' do
Expand Down Expand Up @@ -129,7 +117,6 @@ def perform(succeed: true)
end

it 'sets default values' do
allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)
active_job = ExampleJob.new
adapter.enqueue_at(active_job, nil)

Expand All @@ -138,16 +125,6 @@ def perform(succeed: true)
priority: 0,
scheduled_at: be_within(1.second).of(Time.current)
)

allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
active_job = ExampleJob.new
adapter.enqueue_at(active_job, nil)

expect(GoodJob::Job.last).to have_attributes(
queue_name: 'default',
priority: 0
# scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
)
end
end

Expand Down Expand Up @@ -181,7 +158,6 @@ def perform(succeed: true)
end

it 'sets default values' do
allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)
active_job = ExampleJob.new
adapter.enqueue_all([active_job])

Expand All @@ -190,16 +166,6 @@ def perform(succeed: true)
priority: 0,
scheduled_at: be_within(1.second).of(Time.current)
)

allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
active_job = ExampleJob.new
adapter.enqueue_at(active_job, nil)

expect(GoodJob::Job.last).to have_attributes(
queue_name: 'default',
priority: 0
# scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
)
end

context 'when a job fails to enqueue' do
Expand Down
18 changes: 1 addition & 17 deletions spec/lib/good_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,8 @@ def perform(succeed: true)
end

describe '#v4_ready?' do
it "is true when all jobs are discrete" do
it "is true" do
expect(described_class.v4_ready?).to eq true
end

it "is false when a job is not discrete" do
GoodJob::BaseRecord.uncached do
job = GoodJob::Job.create!(finished_at: nil, is_discrete: false)
expect(described_class.v4_ready?).to eq false

job.update! finished_at: nil, is_discrete: nil
expect(described_class.v4_ready?).to eq false

job.update! finished_at: nil, is_discrete: true
expect(described_class.v4_ready?).to eq true

job.update! finished_at: Time.current, is_discrete: nil
expect(described_class.v4_ready?).to eq true
end
end
end
end

0 comments on commit 5be5c28

Please sign in to comment.