rescue Mixlib::ShellOut::ShellCommandFailed as well - Chef 0.10.10

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Component/s: php
  • Labels:
    None

Description

Eg.
php_pear_channel "pear.symfony-project.com" do
action :discover
end

...fails because in the exists, "channel-info" is used and a Mixlib::ShellOut::ShellCommandFailed is thrown in Chef 0.10.10.

rescue Chef::Exceptions::ShellCommandFailed is required by 0.10.8
rescue Mixlib::ShellOut::ShellCommandFailed is new in 0.10.10
rescue NameError handles the missing Mixlib::ShellOut::ShellCommandFailed for 0.10.8

Issue Links

relates to

Bug - A problem which impairs or prevents the functions of the product. COOK-1282 Python pip provider rescues incorrect error.

  • Unknown - Issue untriaged, priority unassigned or questioned
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
James Brook added a comment -

I have the same trouble. Applying this pull request solves it for me: https://github.com/opscode-cookbooks/php/pull/5

Show
James Brook added a comment - I have the same trouble. Applying this pull request solves it for me: https://github.com/opscode-cookbooks/php/pull/5
Hide
Permalink
Will Milton added a comment -

Me too, `pear channel info` is being caught and allowing my :discover action to fire

Show
Will Milton added a comment - Me too, `pear channel info` is being caught and allowing my :discover action to fire
Hide
Permalink
David Joos added a comment -

This PR solves the issue:
https://github.com/opscode-cookbooks/php/pull/5

Show
David Joos added a comment - This PR solves the issue: https://github.com/opscode-cookbooks/php/pull/5
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

We don't think you need the rescue NameError in there and we'd rather not blindly rescue an base exception. Can you show that you get a NameError there or remove it?

irb(main):001:0> class FooException < Exception
irb(main):002:1> end
=> nil
irb(main):003:0> 
irb(main):004:0* def foo
irb(main):005:1>   begin
irb(main):006:2*     raise FooException 
irb(main):007:2>   rescue FooException
irb(main):008:2>     puts "Got FooException"
irb(main):009:2>   rescue Not::Real::Exception
irb(main):010:2>     puts "Can't get here"
irb(main):011:2>   end
irb(main):012:1> end
=> nil
irb(main):013:0> foo
Got FooException
=> nil
Show
Bryan McLellan [Opscode] added a comment - We don't think you need the rescue NameError in there and we'd rather not blindly rescue an base exception. Can you show that you get a NameError there or remove it? irb(main):001:0> class FooException < Exception irb(main):002:1> end => nil irb(main):003:0> irb(main):004:0* def foo irb(main):005:1> begin irb(main):006:2* raise FooException irb(main):007:2> rescue FooException irb(main):008:2> puts "Got FooException" irb(main):009:2> rescue Not::Real::Exception irb(main):010:2> puts "Can't get here" irb(main):011:2> end irb(main):012:1> end => nil irb(main):013:0> foo Got FooException => nil
Hide
Permalink
David Joos added a comment -

Hi Bryan,

thanks for getting in touch. I ran a couple of tests, and you're right, the rescue NameError is not necessary when running Chef 0.10.12. The only reason of it being there originally was the information in https://github.com/opscode-cookbooks/python/pull/11 as Chef 0.10.8 misses Mixlib::ShellOut::ShellCommandFailed.

I've made the necessary changes in the PR (https://github.com/opscode-cookbooks/php/pull/5) - hopefully it will be possible to get it merged soon.

Thanks in advance!

Kind regards,
David

Show
David Joos added a comment - Hi Bryan, thanks for getting in touch. I ran a couple of tests, and you're right, the rescue NameError is not necessary when running Chef 0.10.12. The only reason of it being there originally was the information in https://github.com/opscode-cookbooks/python/pull/11 as Chef 0.10.8 misses Mixlib::ShellOut::ShellCommandFailed. I've made the necessary changes in the PR ( https://github.com/opscode-cookbooks/php/pull/5 ) - hopefully it will be possible to get it merged soon. Thanks in advance! Kind regards, David
Hide
Permalink
jtimberman added a comment -

Re-resolving this so we pick it up for code review. Thanks for updating!

Show
jtimberman added a comment - Re-resolving this so we pick it up for code review. Thanks for updating!
Hide
Permalink
David Joos added a comment -

Perfect, thanks!

Show
David Joos added a comment - Perfect, thanks!
Hide
Permalink
jtimberman added a comment -

This is now merged. Thanks David!

Show
jtimberman added a comment - This is now merged. Thanks David!
Hide
Permalink
jtimberman added a comment -

Released in v1.1.0

Show
jtimberman added a comment - Released in v1.1.0

People

  • Assignee:
    David Joos
    Reporter:
    David Joos
Vote (3)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: