Skip to content

Commit

Permalink
Addresses PR feedback
Browse files Browse the repository at this point in the history
* Moves defaults handling from MissionControl::Jobs::Application to
  MissionControl::Jobs::Server to minimize impact of this new
  feature on the existing implementation and tests.

  This fixes the unit test failure that I missed in the previous commit.

* Uses @server rather than looking it up via params in the JobsController.
  This allows reverting all changes to the JobsController.
  • Loading branch information
hms committed Oct 16, 2024
1 parent 5e7ed27 commit 927ae8c
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 17 deletions.
9 changes: 0 additions & 9 deletions app/controllers/mission_control/jobs/jobs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ class MissionControl::Jobs::JobsController < MissionControl::Jobs::ApplicationCo
include MissionControl::Jobs::JobScoped, MissionControl::Jobs::JobFilters

skip_before_action :set_job, only: :index
before_action :set_backtrace_cleaner, only: :show

def index
@job_class_names = jobs_with_status.job_class_names
Expand Down Expand Up @@ -38,12 +37,4 @@ def filtered_jobs
def jobs_status
params[:status].presence&.inquiry
end

def set_backtrace_cleaner
@backtrace_cleaner = @application.servers[server_id]&.backtrace_cleaner
end

def server_id
params[:server_id]
end
end
6 changes: 3 additions & 3 deletions app/helpers/mission_control/jobs/jobs_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ def clean_backtrace?
params["clean_backtrace"] == "true"
end

def failed_job_backtrace(job)
if clean_backtrace? && @backtrace_cleaner
@backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
def failed_job_backtrace(job, server)
if clean_backtrace? && server&.backtrace_cleaner
server.backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
else
job.last_execution_error.backtrace.join("\n")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
</tbody>
</table>

<% if @backtrace_cleaner %>
<% if @server.backtrace_cleaner %>
<%= render "mission_control/jobs/jobs/failed/backtrace_toggle", application: @application, job: job %>
<% end %>

<pre class="is-family-monospace mb-4 backtrace-content"><%= failed_job_backtrace(job) %></pre>
<pre class="is-family-monospace mb-4 backtrace-content"><%= failed_job_backtrace(job, @server) %></pre>
<% end %>
1 change: 0 additions & 1 deletion lib/mission_control/jobs/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ def initialize(name:)
def add_servers(queue_adapters_by_name)
queue_adapters_by_name.each do |name, queue_adapter|
adapter, cleaner = queue_adapter
cleaner ||= MissionControl::Jobs.backtrace_cleaner

servers << MissionControl::Jobs::Server.new(name: name.to_s, queue_adapter: adapter,
backtrace_cleaner: cleaner, application: self)
Expand Down
4 changes: 2 additions & 2 deletions lib/mission_control/jobs/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ class MissionControl::Jobs::Server

attr_reader :name, :queue_adapter, :application, :backtrace_cleaner

def initialize(name:, queue_adapter:, application:, backtrace_cleaner:)
def initialize(name:, queue_adapter:, application:, backtrace_cleaner: nil)
super(name: name)
@queue_adapter = queue_adapter
@application = application
@backtrace_cleaner = backtrace_cleaner
@backtrace_cleaner = backtrace_cleaner || MissionControl::Jobs.backtrace_cleaner
end

def activating(&block)
Expand Down

0 comments on commit 927ae8c

Please sign in to comment.