Add execution history for worker #98
Conversation
|
@cookpad/dev-infra Review please. |
| @@ -0,0 +1,44 @@ | |||
| class Kuroko2::WorkerLogsController < Kuroko2::ApplicationController | |||
| def index | |||
| @logs = Kuroko2::WorkerLog.ordered.includes(:job_definition, :job_instance) | |||
There was a problem hiding this comment.
It seems to good to use local variable and not to reassign a value to same variable (to be immutable ).
| end | ||
|
|
||
| def dataset | ||
| @logs = Kuroko2::WorkerLog.ordered.includes(:job_definition, :job_instance) |
| private | ||
|
|
||
| def set_period | ||
| @end_at = begin params[:end_at].to_datetime rescue Time.current end |
There was a problem hiding this comment.
Please specify which error to rescue.
| def set_period | ||
| @end_at = begin params[:end_at].to_datetime rescue Time.current end | ||
|
|
||
| @start_at = begin params[:start_at].to_datetime rescue |
There was a problem hiding this comment.
(ditto) please specify error type
db/migrate/031_create_worker_logs.rb
Outdated
| class CreateWorkerLogs < ActiveRecord::Migration[5.0] | ||
| def change | ||
| create_table :worker_logs do |t| | ||
| t.string :hostname, limit: 180, null: false |
There was a problem hiding this comment.
Doesn't it need to add index to hostname column?
|
|
||
| expect(assigns(:logs)).to match_array logs | ||
| expect(assigns(:end_at)).not_to be_nil | ||
| expect(assigns(:start_at)).to eq assigns(:end_at) - 1.hour |
There was a problem hiding this comment.
[nits]
I think following code is more unambiguous than this code.
expect(assigns(:start_at)).to eq(assigns(:end_at) - 1.hour) | end | ||
| end | ||
|
|
||
| context 'with end_at' do |
There was a problem hiding this comment.
Please add the spec representing if invalid time as end_at and start_at is given.
|
I'm sorry to be late for reviewing 🙇 |
|
@ganmacs Thanks, I fixed reviewed changes. |
| @@ -50,13 +50,24 @@ def invoke(execution) | |||
| command = execution.shell | |||
| env = execution.context.fetch('ENV', {}) | |||
|
|
|||
| worker_log = Kuroko2::WorkerLog.create( | |||
There was a problem hiding this comment.
I worry that a lot of records are made.
You should add a script that cleaning old records like https://github.com/cookpad/kuroko2/blob/b21749d9e9e65211db8656b5fd719e0ec2c601d8/bin/cleanup_old_instances.rb
There was a problem hiding this comment.
I added it in 7cd2bbd.
takonomura
force-pushed
the
feature/worker-logs
branch
from
7cd2bbd to
9a05f39
Compare
takonomura
force-pushed
the
feature/worker-logs
branch
from
9a05f39 to
23071b8
Compare
Add pages to show execution history for worker. You can check executed jobs at specific time.
Worker Logs page
Worker Logs Timeline page