cookpad / kuroko2

Kuroko2 is a web-based job scheduler / workflow engine.
MIT License
314 stars 72 forks source link

Add validation checks for adhoc job launch #79

Closed shigemk2 closed 7 years ago

shigemk2 commented 7 years ago

We expect syntax error when launching adhoc job with wrong syntax scripts in detail page, but we can execute wrong syntax scripts.

screenshot from 2017-07-21 00-07-45

So, I add syntax validation check for adhoc job launch and show error messages in modal.

screenshot from 2017-07-21 00-08-03

eisuke commented 7 years ago

but we can execute wrong syntax scripts

Kuroko2 raises error when the wrong syntax scripts are inputed.

kuroko2_script_error

Is not this enough?

shigemk2 commented 7 years ago

In kuroko2 0.4.2 and local development, the adhoc job with wrong scripts would hang(it remains "working") and application would crash.

Screenshot:

2017-07-21 16 56 33

Error log(fixed path and worker name):

16:41:55 processor.1 | F, [2017-07-21T16:41:55.355669 #79619] FATAL -- : [worker-0] ArgumentError: wrong number of arguments (given 1, expected 0)
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/relation.rb:492:in `delete_all'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/querying.rb:8:in `delete_all'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:124:in `rescue in process_with_lock'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:131:in `process_with_lock'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:17:in `block in process'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/locking/pessimistic.rb:81:in `block in with_lock'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/database_statements.rb:225:in `block in transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `block in within_new_transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/2.4.0/monitor.rb:214:in `mon_synchronize'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/transaction.rb:191:in `within_new_transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/connection_adapters/abstract/database_statements.rb:225:in `transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/transactions.rb:210:in `transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/transactions.rb:299:in `transaction'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/locking/pessimistic.rb:79:in `with_lock'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:17:in `process'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:6:in `block in process_all'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/relation/delegation.rb:39:in `each'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activerecord-5.1.2/lib/active_record/relation/delegation.rb:39:in `each'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/engine.rb:5:in `process_all'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/workflow/processor.rb:21:in `run'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/serverengine-1.5.11/lib/serverengine/worker.rb:74:in `main'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/serverengine-1.5.11/lib/serverengine/embedded_server.rb:24:in `run'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/serverengine-1.5.11/lib/serverengine/server.rb:88:in `main'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/serverengine-1.5.11/lib/serverengine/daemon.rb:101:in `main'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/serverengine-1.5.11/lib/serverengine/daemon.rb:90:in `run'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/kuroko2-0.4.2/lib/autoload/kuroko2/servers/base.rb:16:in `run'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/commands/runner/runner_command.rb:37:in `perform'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/commands/runner/runner_command.rb:37:in `eval'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/commands/runner/runner_command.rb:37:in `perform'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/thor-0.19.4/lib/thor/command.rb:27:in `run'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/thor-0.19.4/lib/thor/invocation.rb:126:in `invoke_command'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/thor-0.19.4/lib/thor.rb:369:in `dispatch'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/command/base.rb:63:in `perform'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/command.rb:44:in `invoke'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/railties-5.1.2/lib/rails/commands.rb:16:in `<top (required)>'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:292:in `require'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:292:in `block in require'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:258:in `load_dependency'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:292:in `require'
16:41:55 processor.1 |     /Users/user/src/kuroko2_test_3/bin/rails:9:in `<top (required)>'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:286:in `load'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:286:in `block in load'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:258:in `load_dependency'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/activesupport-5.1.2/lib/active_support/dependencies.rb:286:in `load'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
16:41:55 processor.1 |     /Users/user/.rbenv/versions/2.4.1/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
16:41:55 processor.1 |     -e:1:in `<main>'
shigemk2 commented 7 years ago

I found job was not running, so I think system should validate scripts when launching adhoc jobs.

eisuke commented 7 years ago

@shigemk2 Fixed cause in https://github.com/cookpad/kuroko2/pull/80 . I want to keep behavior like https://github.com/cookpad/kuroko2/pull/79#issuecomment-316908304 when the invalid script is inputted. Thank you for reporting.