facebook / grocery-delivery

The Grocery Delivery utility for managing cookbook uploads to distributed Chef backends.
Apache License 2.0
154 stars 55 forks source link

GD returns false positive exit status #34

Closed krzkowalczyk closed 6 years ago

krzkowalczyk commented 7 years ago

Hi,

Summary

Due to rescue block used for upload_changed GD writes errors to stdout, but always returns success on exit status. $success variable stores exit code, but it's not used. This affects validating GD execution.

Details

False positive example

Proposed solution

I've been using GD triggered by commit hook, so I fixed it by adding simple exit($success) at the end, in order to be able to evaluate the run. You can find it here

Therefore, I'm not 100% sure if it should go there, as we have post hooks as well. The question is if the postrun hook should be executed when there was no success with knife upload, and if atexit hook shouldn't be the place to return status code.

Regards, Krzysztof

jaymzh commented 7 years ago

Your change seems correct to me. We run it in cron on all of our chefservers instead of in a hook, so having it exit unsuccessfully in our case isn't desired since we'd just get spammed with a ton of email and we already monitor the status of this in the postrun hook.

So we'll need to do like a "grocery-delivery;true" but I think that's fine, the correct behavior is clearly to have valid exit codes.

Wanna send a PR?