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

Fix chef-server-ctl backup always returning 1 #537

Merged
merged 1 commit into from
Sep 23, 2015
Merged

Conversation

ryancragun
Copy link
Contributor

@stevendanna
Copy link
Contributor

I'm 👍 on just merging; however, if you have a few minutes, here are some things I had questions about:

  1. Error output should probably go to standard error rather than standard out.
  2. Given our purpose, I think the bare rescue makes sense, but I'd prefer to see it just around foreign code (ChefBackup::Runner). The other code we are wrapping looks to be related to transforming user-input into configuration. If something is wrong with those cases, I think we are going to want something more descriptive than the ruby exception message since it probably means the user has input they need to fix.


status = ChefBackup::Runner.new(running_config).backup
exit(status ? 0 : 1)
exit(0)
Copy link
Member

Choose a reason for hiding this comment

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

the way omnibus-ctl works you can just 'return 0' , and above 'return 1'. That will be used as the exit code.

@marcparadise
Copy link
Member

@stevendanna to the best of my recollection, all of the omnibus-ctl error output (except wrapped chef runs) go to stdout.

@ryancragun
Copy link
Contributor Author

@stevendanna All error output now goes through STDERR and we only rescue on the foreign code.
@marcparadise I've seen early returns in omnibus-ctl cause jump exceptions because omnibus-ctl will try and yield a non-existent block, hence the specific exits.

@stevendanna
Copy link
Contributor

Thanks for the updates, 👍 from me. Will merge at EOD if there are no objections

stevendanna added a commit that referenced this pull request Sep 23, 2015
Fix chef-server-ctl backup always returning 1
@stevendanna stevendanna merged commit 25ea146 into master Sep 23, 2015
@ryancragun ryancragun deleted the ryan/backup_exit branch October 19, 2015 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants