-
Notifications
You must be signed in to change notification settings - Fork 93
Feature: Add optional output to maintenance task to display output #1251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
09139ec
to
5bb6f0c
Compare
5bb6f0c
to
d4c7c01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use Rails.logger
in your task? Do we need to persist log data to the run itself? This seems like it would get out of hand pretty fast on tasks that execute on thousands of records...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to agree with Adrianna, in our experience this solution will not sustain in tasks that iterate over millions of records. And looking at the examples, from my personal experience I wouldn't even recommend issuing logs such as Marking user for cleanup
or Successfully processed
, those things should be implied by the "green path" of the task, meaning that if no error happened and the iteration counter went up it should be safe to assume that the task did what it was supposed to do. Otherwise a log filled with entries like "business as usual" becomes extremely hard to utilize for debugging purposes or just gets truncated depending on the infra setup
return unless self.class.column_names.include?("output") | ||
|
||
# Reload output from database to get the latest value | ||
current_output = self.class.where(id: id).pluck(:output).first || "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I don't think we can afford adding a feature like that to the framework. It may work okay for smaller applications but for large applications with a lot of rows to iterate over the approach of transferring large chunks for data back-and-forth doesn't scale well
Thanks everyone for the feedback!
I understand your concern about larger applications with multiple rows touching the database. Another idea I had was rather than writing to the database was writing the output to |
Add Optional Output Logging for Maintenance Tasks
Summary
This PR introduces an optional output logging feature for maintenance tasks, allowing
tasks to capture and display detailed execution logs in the web UI. The output is
stored persistently in the database and displayed in real-time during task execution.
Why This Change?
Maintenance tasks often perform complex operations on large datasets. Previously,
there was no built-in way to track what the task was doing beyond basic progress
metrics. This feature enables:
Implementation Details
Database Changes
output
text column tomaintenance_tasks_runs
table via migrationCore Functionality
log_output(message)
method toMaintenanceTasks::Task
base classappend_output(new_output)
method toMaintenanceTasks::Run
modelUI Changes
_output.html.erb
partial to display task outputUsage Example
Visual Example
Testing
edge cases
Documentation
Backward Compatibility
Next Steps
Users need to run the migration to enable this feature:
rails generate migration AddOutputToMaintenanceTasksRuns output:text
rails db:migrate