From 508133d3f7aa9b18baa08e9886f2a8b137a8a1e4 Mon Sep 17 00:00:00 2001 From: Ben Sheldon Date: Mon, 13 Sep 2021 07:17:14 -0700 Subject: [PATCH] Rename `GoodJob::Job` to `GoodJob::Execution` - Also renames `GoodJob::CurrentExecution` to `GoodJob::CurrentThread` - Updates Dashboard controllers and views/partials; puts the "Active Job ID" column first in the table view; wraps UUIDs with `` tags --- .../good_job/active_jobs_controller.rb | 5 +- .../good_job/dashboards_controller.rb | 33 +- .../good_job/executions_controller.rb | 10 + .../controllers/good_job/jobs_controller.rb | 10 - .../views/good_job/active_jobs/show.html.erb | 4 +- .../good_job/cron_schedules/index.html.erb | 2 +- .../views/good_job/dashboards/index.html.erb | 10 +- .../good_job/shared/_executions_table.erb | 56 +++ .../app/views/good_job/shared/_jobs_table.erb | 48 -- .../app/views/layouts/good_job/base.html.erb | 2 +- engine/config/routes.rb | 2 +- lib/good_job.rb | 4 +- .../active_job_extensions/concurrency.rb | 12 +- lib/good_job/adapter.rb | 16 +- lib/good_job/cron_manager.rb | 6 +- ...current_execution.rb => current_thread.rb} | 16 +- lib/good_job/execution.rb | 308 ++++++++++++ lib/good_job/job.rb | 300 +----------- lib/good_job/job_performer.rb | 4 +- lib/good_job/log_subscriber.rb | 8 +- lib/good_job/notifier.rb | 6 +- lib/good_job/railtie.rb | 4 +- lib/good_job/scheduler.rb | 4 +- scripts/benchmark_job_throughput.rb | 16 +- spec/app/jobs/example_job_spec.rb | 22 +- spec/integration/adapter_spec.rb | 18 +- spec/integration/scheduler_spec.rb | 18 +- .../active_job_extensions/concurrency_spec.rb | 14 +- spec/lib/good_job/adapter_spec.rb | 16 +- spec/lib/good_job/cli_spec.rb | 8 +- spec/lib/good_job/cron_manager_spec.rb | 6 +- ...ecution_spec.rb => current_thread_spec.rb} | 8 +- spec/lib/good_job/execution_spec.rb | 455 ++++++++++++++++++ spec/lib/good_job/job_performer_spec.rb | 4 +- spec/lib/good_job/job_spec.rb | 453 +---------------- spec/lib/good_job/lockable_spec.rb | 142 +++--- spec/lib/good_job_spec.rb | 6 +- spec/support/reset_good_job.rb | 6 +- spec/system/dashboard_spec.rb | 4 +- spec/test_app/app/jobs/cleanup_job.rb | 4 +- ..._and_concurrency_key_index_to_good_jobs.rb | 4 +- spec/test_app/db/seeds.rb | 4 +- 42 files changed, 1061 insertions(+), 1017 deletions(-) create mode 100644 engine/app/controllers/good_job/executions_controller.rb delete mode 100644 engine/app/controllers/good_job/jobs_controller.rb create mode 100644 engine/app/views/good_job/shared/_executions_table.erb delete mode 100644 engine/app/views/good_job/shared/_jobs_table.erb rename lib/good_job/{current_execution.rb => current_thread.rb} (85%) create mode 100644 lib/good_job/execution.rb rename spec/lib/good_job/{current_execution_spec.rb => current_thread_spec.rb} (86%) create mode 100644 spec/lib/good_job/execution_spec.rb diff --git a/engine/app/controllers/good_job/active_jobs_controller.rb b/engine/app/controllers/good_job/active_jobs_controller.rb index 1a4255caf..0dbb55b4b 100644 --- a/engine/app/controllers/good_job/active_jobs_controller.rb +++ b/engine/app/controllers/good_job/active_jobs_controller.rb @@ -2,8 +2,9 @@ module GoodJob class ActiveJobsController < GoodJob::BaseController def show - @jobs = GoodJob::Job.where("serialized_params ->> 'job_id' = ?", params[:id]) - .order(Arel.sql("COALESCE(scheduled_at, created_at) DESC")) + @executions = GoodJob::Execution.active_job_id(params[:id]) + .order(Arel.sql("COALESCE(scheduled_at, created_at) DESC")) + raise ActiveRecord::RecordNotFound if @executions.empty? end end end diff --git a/engine/app/controllers/good_job/dashboards_controller.rb b/engine/app/controllers/good_job/dashboards_controller.rb index 3182d509b..cce0a3bd3 100644 --- a/engine/app/controllers/good_job/dashboards_controller.rb +++ b/engine/app/controllers/good_job/dashboards_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module GoodJob class DashboardsController < GoodJob::BaseController - class JobFilter + class ExecutionFilter attr_accessor :params def initialize(params) @@ -9,14 +9,14 @@ def initialize(params) end def last - @_last ||= jobs.last + @_last ||= executions.last end - def jobs + def executions after_scheduled_at = params[:after_scheduled_at].present? ? Time.zone.parse(params[:after_scheduled_at]) : nil - sql = GoodJob::Job.display_all(after_scheduled_at: after_scheduled_at, after_id: params[:after_id]) - .limit(params.fetch(:limit, 25)) - sql = sql.with_job_class(params[:job_class]) if params[:job_class] + sql = GoodJob::Execution.display_all(after_scheduled_at: after_scheduled_at, after_id: params[:after_id]) + .limit(params.fetch(:limit, 25)) + sql = sql.job_class(params[:job_class]) if params[:job_class] if params[:state] case params[:state] when 'finished' @@ -34,15 +34,16 @@ def jobs def states { - 'finished' => GoodJob::Job.finished.count, - 'unfinished' => GoodJob::Job.unfinished.count, - 'running' => GoodJob::Job.running.count, - 'errors' => GoodJob::Job.where.not(error: nil).count, + 'finished' => GoodJob::Execution.finished.count, + 'unfinished' => GoodJob::Execution.unfinished.count, + 'running' => GoodJob::Execution.running.count, + 'errors' => GoodJob::Execution.where.not(error: nil).count, } end def job_classes - GoodJob::Job.group("serialized_params->>'job_class'").count + GoodJob::Execution.group("serialized_params->>'job_class'").count + .sort_by { |name, _count| name } end def to_params(override) @@ -54,9 +55,9 @@ def to_params(override) end def index - @filter = JobFilter.new(params) + @filter = ExecutionFilter.new(params) - count_query = Arel.sql(GoodJob::Job.pg_or_jdbc_query(<<~SQL.squish)) + count_query = Arel.sql(GoodJob::Execution.pg_or_jdbc_query(<<~SQL.squish)) SELECT * FROM generate_series( date_trunc('hour', $1::timestamp), @@ -81,11 +82,11 @@ def index current_time = Time.current binds = [[nil, current_time - 1.day], [nil, current_time]] - job_data = GoodJob::Job.connection.exec_query(count_query, "GoodJob Dashboard Chart", binds) + executions_data = GoodJob::Execution.connection.exec_query(count_query, "GoodJob Dashboard Chart", binds) - queue_names = job_data.map { |d| d['queue_name'] }.uniq + queue_names = executions_data.map { |d| d['queue_name'] }.uniq labels = [] - queues_data = job_data.to_a.group_by { |d| d['timestamp'] }.each_with_object({}) do |(timestamp, values), hash| + queues_data = executions_data.to_a.group_by { |d| d['timestamp'] }.each_with_object({}) do |(timestamp, values), hash| labels << timestamp.in_time_zone.strftime('%H:%M %z') queue_names.each do |queue_name| (hash[queue_name] ||= []) << values.find { |d| d['queue_name'] == queue_name }&.[]('count') diff --git a/engine/app/controllers/good_job/executions_controller.rb b/engine/app/controllers/good_job/executions_controller.rb new file mode 100644 index 000000000..1689cb85b --- /dev/null +++ b/engine/app/controllers/good_job/executions_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true +module GoodJob + class ExecutionsController < GoodJob::BaseController + def destroy + deleted_count = GoodJob::Execution.where(id: params[:id]).delete_all + message = deleted_count.positive? ? { notice: "Job execution deleted" } : { alert: "Job execution not deleted" } + redirect_to root_path, **message + end + end +end diff --git a/engine/app/controllers/good_job/jobs_controller.rb b/engine/app/controllers/good_job/jobs_controller.rb deleted file mode 100644 index ede1f23c0..000000000 --- a/engine/app/controllers/good_job/jobs_controller.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true -module GoodJob - class JobsController < GoodJob::BaseController - def destroy - deleted_count = GoodJob::Job.where(id: params[:id]).delete_all - message = deleted_count.positive? ? { notice: "Job deleted" } : { alert: "Job not deleted" } - redirect_to root_path, **message - end - end -end diff --git a/engine/app/views/good_job/active_jobs/show.html.erb b/engine/app/views/good_job/active_jobs/show.html.erb index e23da5fae..c428a4714 100644 --- a/engine/app/views/good_job/active_jobs/show.html.erb +++ b/engine/app/views/good_job/active_jobs/show.html.erb @@ -1 +1,3 @@ -<%= render 'good_job/shared/jobs_table', jobs: @jobs %> +

ActiveJob ID: <%= @executions.first.id %>

+ +<%= render 'good_job/shared/executions_table', executions: @executions %> diff --git a/engine/app/views/good_job/cron_schedules/index.html.erb b/engine/app/views/good_job/cron_schedules/index.html.erb index 1991add66..595f4e1b5 100644 --- a/engine/app/views/good_job/cron_schedules/index.html.erb +++ b/engine/app/views/good_job/cron_schedules/index.html.erb @@ -7,7 +7,7 @@ Configuration Class Description - Next execution + Next scheduled <% @cron_schedules.each do |job_key, job| %> diff --git a/engine/app/views/good_job/dashboards/index.html.erb b/engine/app/views/good_job/dashboards/index.html.erb index ba9f1fab9..a36c9f998 100644 --- a/engine/app/views/good_job/dashboards/index.html.erb +++ b/engine/app/views/good_job/dashboards/index.html.erb @@ -7,7 +7,7 @@
Filter by job class
- <% @filter.job_classes.each do |name, count| %> + <% @filter.job_classes.each do |(name, count)| %> <% if params[:job_class] == name %> <%= link_to(root_path(@filter.to_params(job_class: nil)), class: 'btn btn-sm btn-outline-secondary active', role: "button", "aria-pressed": true) do %> <%= name %> (<%= count %>) @@ -37,18 +37,18 @@
-<% if @filter.jobs.present? %> - <%= render 'good_job/shared/jobs_table', jobs: @filter.jobs %> +<% if @filter.executions.present? %> + <%= render 'good_job/shared/executions_table', executions: @filter.executions %> <% else %> - No jobs present. + No executions present. <% end %> diff --git a/engine/app/views/good_job/shared/_executions_table.erb b/engine/app/views/good_job/shared/_executions_table.erb new file mode 100644 index 000000000..818730e13 --- /dev/null +++ b/engine/app/views/good_job/shared/_executions_table.erb @@ -0,0 +1,56 @@ +
+
+ + + + + + + + + + + + + + + <% executions.each do |execution| %> + + + + + + + + + + + <% end %> + +
ActiveJob IDExecution IDJob ClassQueueScheduled AtError + ActiveJob Params  + <%= tag.button "Toggle", type: "button", class: "btn btn-sm btn-outline-primary", role: "button", + data: { bs_toggle: "collapse", bs_target: ".job-params" }, + aria: { expanded: false, controls: executions.map { |execution| "##{dom_id(execution, "params")}" }.join(" ") } + %> + Actions
+ <%= link_to active_job_path(execution.serialized_params['job_id']) do %> + <%= execution.active_job_id %> + <% end %> + + <%= link_to active_job_path(execution.active_job_id, anchor: dom_id(execution)) do %> + <%= execution.id %> + <% end %> + <%= execution.serialized_params['job_class'] %><%= execution.queue_name %><%= execution.scheduled_at || execution.created_at %><%= truncate(execution.error, length: 1_000) %> + <%= tag.button "Preview", type: "button", class: "btn btn-sm btn-outline-primary", role: "button", + data: { bs_toggle: "collapse", bs_target: "##{dom_id(execution, 'params')}" }, + aria: { expanded: false, controls: dom_id(execution, "params") } + %> + <%= tag.pre JSON.pretty_generate(execution.serialized_params), id: dom_id(execution, "params"), class: "collapse job-params" %> + + <%= button_to execution_path(execution.id), method: :delete, class: "btn btn-sm btn-outline-danger", title: "Delete execution" do %> + <%= render "good_job/shared/icons/trash" %> + <% end %> +
+
+
diff --git a/engine/app/views/good_job/shared/_jobs_table.erb b/engine/app/views/good_job/shared/_jobs_table.erb deleted file mode 100644 index 58c725f60..000000000 --- a/engine/app/views/good_job/shared/_jobs_table.erb +++ /dev/null @@ -1,48 +0,0 @@ -
-
- - - - - - - - - - - - - - - <% jobs.each do |job| %> - - - - - - - - - - - <% end %> - -
GoodJob IDActiveJob IDJob ClassQueueScheduled AtError - ActiveJob Params  - <%= tag.button "Toggle", type: "button", class: "btn btn-sm btn-outline-primary", role: "button", - data: { bs_toggle: "collapse", bs_target: ".job-params" }, - aria: { expanded: false, controls: jobs.map { |job| "##{dom_id(job, "params")}" }.join(" ") } - %> - Actions
<%= link_to job.id, active_job_path(job.serialized_params['job_id'], anchor: dom_id(job)) %><%= link_to job.serialized_params['job_id'], active_job_path(job.serialized_params['job_id']) %><%= job.serialized_params['job_class'] %><%= job.queue_name %><%= job.scheduled_at || job.created_at %><%= truncate(job.error, length: 1_000) %> - <%= tag.button "Preview", type: "button", class: "btn btn-sm btn-outline-primary", role: "button", - data: { bs_toggle: "collapse", bs_target: "##{dom_id(job, 'params')}" }, - aria: { expanded: false, controls: dom_id(job, "params") } - %> - <%= tag.pre JSON.pretty_generate(job.serialized_params), id: dom_id(job, "params"), class: "collapse job-params" %> - - <%= button_to job_path(job.id), method: :delete, class: "btn btn-sm btn-outline-danger", title: "Delete job" do %> - <%= render "good_job/shared/icons/trash" %> - <% end %> -
-
-
diff --git a/engine/app/views/layouts/good_job/base.html.erb b/engine/app/views/layouts/good_job/base.html.erb index 09d24e179..466c3e7c7 100644 --- a/engine/app/views/layouts/good_job/base.html.erb +++ b/engine/app/views/layouts/good_job/base.html.erb @@ -23,7 +23,7 @@