-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix job not being recorded as completed when exit status > 127 #55
Fix job not being recorded as completed when exit status > 127 #55
Conversation
@@ -0,0 +1,9 @@ | |||
class ChangeExitStatusToUnsignedTinyint < ActiveRecord::Migration | |||
def up | |||
change_column :executions, :exit_status, 'tinyint(4) unsigned' |
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.
AR migration should be database independent. You might want to use limit: 2
instead of unsigned
.
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.
Makes sense, fixed in ff272e3
spec/dummy/db/schema.rb
Outdated
@@ -182,7 +182,7 @@ | |||
t.datetime "suspended_at" | |||
t.datetime "created_at" | |||
t.datetime "updated_at" | |||
t.index ["email"], name: "email", unique: true, using: :btree | |||
t.index ["email"], name: "email", using: :btree |
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.
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.
Oops...
Fixed 0382965
Please rebase or merge the master.
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.
Thanks. I just rebased on master ddb10ea
Kuroko would fail to record the completion of the job when it exits with status bigger than 127 as it causes an ActiveModel::RangeError.
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.
CI failures will be resolved by #56 .
No problem merging this changes 🙏
Thanks! The #56 changes accept it. |
Thank you for reviewing and merging! |
Kuroko would fail to record the completion of the job when it exits
with status bigger than 127 as it causes an ActiveModel::RangeError.