Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

throw errors from index #89

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

Add the ability to throw errors on an index failure.

Owner

pat commented May 4, 2014

So, I've cherry-picked this into the feature/throw-errors branch, tweaked a few things slightly, but I'm keen to edit further (or you can too). I'll commit on the lines in question so the context is a bit more obvious.

@pat pat commented on the diff May 4, 2014

lib/riddle/controller.rb
@@ -23,7 +23,13 @@ def index(*indices)
cmd = "#{indexer} --config \"#{@path}\" #{indices.join(' ')}"
cmd << " --rotate" if running?
- options[:verbose] ? system(cmd) : `#{cmd}`
+ cmd << " 2>&1"
@pat

pat May 4, 2014

Owner

I don't think this will work on Windows (but happy to be proven wrong).

@pat pat commented on the diff May 4, 2014

lib/riddle/controller.rb
@@ -23,7 +23,13 @@ def index(*indices)
cmd = "#{indexer} --config \"#{@path}\" #{indices.join(' ')}"
cmd << " --rotate" if running?
- options[:verbose] ? system(cmd) : `#{cmd}`
+ cmd << " 2>&1"
+ output = `#{cmd}`
+ logger output if options[:verbose]
@pat

pat May 4, 2014

Owner

It's much nicer to call out to system so the feedback is automatic, rather than waiting for the call to finish and then printing all output all at once. Maybe we can make use of a different way of shelling out, handle STDOUT/STDERR in a smarter way?

@pat pat commented on the diff May 4, 2014

lib/riddle/controller.rb
@@ -23,7 +23,13 @@ def index(*indices)
cmd = "#{indexer} --config \"#{@path}\" #{indices.join(' ')}"
cmd << " --rotate" if running?
- options[:verbose] ? system(cmd) : `#{cmd}`
+ cmd << " 2>&1"
+ output = `#{cmd}`
+ logger output if options[:verbose]
+ errors = /^ERROR: (.*)$/.match(output).to_a
+ error = errors[0] unless errors.empty?
@pat

pat May 4, 2014

Owner

Perhaps easier to call error = output[/^ERROR: (.*)$/] instead of these two lines?

Owner

pat commented May 4, 2014

Oh, and the reasoning for putting this in a feature branch... firstly, I run Riddle and TS both with the git-flow approach (so, larger/breaking features get fleshed out in feature branches until they're ready, then they're merged into develop. The develop branch is where upcoming changes sit before a gem release, and then they get merged into master).

This may end up being a significant change, hence it's not put in develop straight away.

Useful links on git-flow, if you're not familiar with it:
http://jeffkreeftmeijer.com/2010/why-arent-you-using-git-flow/
http://danielkummer.github.io/git-flow-cheatsheet/

Anyway, a few things to ponder - keen to hear your feedback :)

Owner

pat commented Aug 4, 2014

So, I've spent some time investigating this over the weekend, and talked it through with a few others, and I can't find a way that will get live output (which is what system does) and capture the STDOUT/STDERR into Ruby as well (like backticks) on both *nix and Windows without major hackery (if it is indeed possible).

Hence, I'm going to close this ticket, which makes me sad. If someone can find a way through, wonderful, but I don't think it's worth the time - and I don't want to add particularly complex code, as that makes supporting changes further down the track painful.

@pat pat closed this Aug 4, 2014

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