Don't use rails runner to launch scripts, require config/environment #4307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| runner = File.join(rr, "bin/rails runner") | ||
| program = File.join(rr, "lib/workers/bin/evm_server.rb") | ||
| command_line = "#{runner} #{program}" | ||
| command_line = "#{Gem.ruby} #{Rails.root.join("lib/workers/bin/evm_server.rb").expand_path}" |
| @@ -1,3 +1,5 @@ | |||
| require File.expand_path("../../../config/environment", __dir__) | |||
|
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
jrafanie
force-pushed
the
dont_use_rails_runner_to_launch_scripts
branch
from
75907c8 to
b2fdb51
Compare
The primary use cases for rails runner are for * quick in-line ruby evaluation such as `rails runner 'puts User.first.name'` * shebang line at the top of executables While it supports passing the path to a ruby script, it leads to weird errors if the file is not found for any reason: `$ bin/rails r "/home/joe/run.rb"` => `unknown regexp option - j (SyntaxError)` `$ bin/rails r "joe/run.rb"` => `undefined local variable or method `joe' for main:Object (NameError)` Since this is NOT the typical use case for rails runner, it's safer to use ruby to load our launcher and require config/environment in the launcher. A nice side effect is $PROGRAM_NAME can be reliably used to detect what type of worker the current process is: `manageiq/lib/workers/bin/evm_server.rb`
jrafanie
force-pushed
the
dont_use_rails_runner_to_launch_scripts
branch
from
b2fdb51 to
cfa3603
Compare
|
Checked commit jrafanie@cfa3603 with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0 |
jrafanie
changed the title
|
@miq-bot rm_label wip |
| require "workers/evm_server" | ||
| EvmServer.start(*ARGV) | ||
| else | ||
| puts "run with rails runner evm_server.rb" |
There was a problem hiding this comment.
@matthewd I removed the lies 👆
|
👍 can't wait till it turns green |
|
@matthewd Please review 🍰 |
matthewd
added a commit
that referenced
this pull request
Jan 27, 2016
…h_scripts Don't use rails runner to launch scripts, require config/environment
|
👏 |
|
Yay! Great work mr lj |
|
Yay, thanks!!! 🎉 |
jrafanie
added a commit
to jrafanie/manageiq
that referenced
this pull request
Sep 20, 2016
Benefits: * rails runner is for shebang lines or inline ruby, see ManageIQ#4307 * We don't have to check if rails is loaded. * Can be run outside rails root directory via ruby. * We don't need -- to avoid rails runner modifying our args. * Less typing. * Existing bin/rails runner callers with args just work: `ruby tools/purge_orphaned_tag_values.rb -s 100` Or, the old way: `bin/rails runner tools/purge_orphaned_tag_values.rb -- -s 100` Discovered while reviewing: ManageIQ#11281
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will require a bit more work to get the workers to launch properly and to ensure theMiqEnvironment::Process.is_*?methods can understand workers and the server no longer launching via rails runner.Update: MiqEnvironment::Process is gone 🎉 , see #3593
The primary use cases for rails runner are for
rails runner 'puts User.first.name'While it supports passing the path to a ruby script, it leads to weird
errors if the file is not found for any reason:
$ bin/rails r "/home/joe/run.rb"=>
unknown regexp option - j (SyntaxError)$ bin/rails r "joe/run.rb"=>
undefined local variable or methodjoe' for main:Object (NameError)`Since this is NOT the typical use case for rails runner, it's safer to
use ruby to load our launcher and require config/environment in the
launcher.
A nice side effect is $PROGRAM_NAME can be reliably used to detect what
type of worker the current process is:
manageiq/lib/workers/bin/evm_server.rb