maintenance_tasks icon indicating copy to clipboard operation
maintenance_tasks copied to clipboard

Expose Run instance on Task

Open olivier-thatch opened this issue 1 year ago • 2 comments

Expose Run instance on Task.

This is basically the same as #924, so all credit goes to @shalvah. I'm opening a new PR since the old one was closed and I'm hoping a new PR will be more visible to the maintainers (but also happy to close this PR in favor of the old one).

We've been using this in our codebase via the following monkey patch:

require "maintenance_tasks"

module MaintenanceTasks
  class Run < ApplicationRecord
    module SetTaskRunPatch
      def task
        super.tap do |task|
          task.run ||= self
        end
      end
    end

    prepend SetTaskRunPatch
  end

  class Task
    attr_accessor :run
  end
end

This enables us to do some interesting stuff like exposing a logger to tasks and storing the log output on the Run instances, or adding the run ID to our error contexts, etc.

olivier-thatch avatar May 15 '24 22:05 olivier-thatch

@etiennebarrie Sorry for the ping but would love to get some feedback on this PR :)

olivier-thatch avatar Jun 11 '24 21:06 olivier-thatch

Our stance has been not to expose Run itself, so that it stays an internal concern, and solving the actual issues directly. For example:

adding the run ID to our error contexts

Since we added #941, we should probably also add the id to the error task context: https://github.com/Shopify/maintenance_tasks/blob/93a50672c36a26e3a0954bb2deb50ab211b100f7/app/jobs/concerns/maintenance_tasks/task_job_concern.rb#L173-L177

etiennebarrie avatar Jun 12 '24 10:06 etiennebarrie