Skip to content
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

Add a Rake.application.running? predicate #243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leonid-shevtsov
Copy link

@leonid-shevtsov leonid-shevtsov commented Dec 19, 2017

I have added a Rake.application.running? method, that returns true if Rake is running currently, and false otherwise. It is most useful to fence off code that is only needed for Rake tasks, or conversely, should not run inside a Rake task.

This is a common need in Rails apps and the most popular way to see if the code is running within a rake task is checking the command line argument (see SO answers at the bottom). Which might work, but it is messy, and leads to copy-paste of the File.basename($0) == 'rake' idiom. Our project uses that code snippet in three distinct places, but instead of fixing this on a project level, in my opinion, it is a capability that Rake should have out of the box.

StackOverflow answers that suggest checking the script filename:

@yuki24
Copy link
Member

yuki24 commented Dec 19, 2017

It is most useful to fence off code that is only needed for Rake tasks, or conversely, should not run inside a Rake task.

Interesting, I haven't had the need like this. Could you elaborate on this?

standard_exception_handling do
init "rake", argv
load_rakefile
top_level
end
@running = false
Copy link
Member

@yuki24 yuki24 Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be wrapped with an ensure block:

def run(...)
  ..
ensure
  @running = false
end

assert !@app.running?
assert was_running
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test case where the task execution fails but it correctly sets @running back to false after failure?

@leonid-shevtsov
Copy link
Author

Done - sorry for the huge delay.

To elaborate on this:

It is most useful to fence off code that is only needed for Rake tasks, or conversely, should not run inside a Rake task.

to give one example, we mark changes made from a rake task for audit:

if File.basename($PROGRAM_NAME) == "rake"
  PaperTrail.whodunnit = "#{`whoami`.strip}: rake #{ARGV.join ' '}"
end

Another places I can recall:

  • extended logging inside Rake tasks - logs that would be spammy in the server process.
  • do not start project-specific subsystems that are known to make Rake tasks slow and are not necessary in Rake tasks.

(All of the above assumed a webapp context.)

@booch
Copy link

booch commented Aug 30, 2018

I'm the author of one of those Stack Overflow questions (many years ago).

I think this is a worthwhile addition to Rake, but I have 1 suggestion and 1 concern:

First, I think it would be simpler to just have it be Rake.running?. Second, to use this, you'd have to check that the Rake constant is defined:

defined?(Rake) && Rake.running?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants