{"action":"created","issue":{"url":"https://api.github.com/repos/genome/genome/issues/799","labels_url":"https://api.github.com/repos/genome/genome/issues/799/labels{/name}","comments_url":"https://api.github.com/repos/genome/genome/issues/799/comments","events_url":"https://api.github.com/repos/genome/genome/issues/799/events","html_url":"https://github.com/genome/genome/pull/799","id":85280738,"number":799,"title":"Ensure that _teardown_logging happens","user":{"login":"mkiwala","id":108881,"avatar_url":"https://avatars.githubusercontent.com/u/108881?v=3","gravatar_id":"","url":"https://api.github.com/users/mkiwala","html_url":"https://github.com/mkiwala","followers_url":"https://api.github.com/users/mkiwala/followers","following_url":"https://api.github.com/users/mkiwala/following{/other_user}","gists_url":"https://api.github.com/users/mkiwala/gists{/gist_id}","starred_url":"https://api.github.com/users/mkiwala/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mkiwala/subscriptions","organizations_url":"https://api.github.com/users/mkiwala/orgs","repos_url":"https://api.github.com/users/mkiwala/repos","events_url":"https://api.github.com/users/mkiwala/events{/privacy}","received_events_url":"https://api.github.com/users/mkiwala/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-06-04T21:15:02Z","updated_at":"2015-06-05T04:58:53Z","closed_at":"2015-06-05T04:58:53Z","pull_request":{"url":"https://api.github.com/repos/genome/genome/pulls/799","html_url":"https://github.com/genome/genome/pull/799","diff_url":"https://github.com/genome/genome/pull/799.diff","patch_url":"https://github.com/genome/genome/pull/799.patch"},"body":"If `_setup_logging()` is called and succeeds, then the `_teardown_logging()` function must be called or the wrapper will perpetually hang while waiting for annotate-log processes to spontaneously end (they never will).\r\n\r\nBefore this PR, try/catch was being used to safely handle the command-class, and then exceptions were being re-thrown with `croak()` to give a stack trace.  It was possible for `execute()` to exit early if `_instantiate_command()` or `_run_command()` threw an exception.  The re-thrown exception would filter up past `execute()` without doing `_teardown_logging()` first.\r\n\r\nIn place of `croak()`, this PR uses `cluck()` to log a stack trace at the point of catching the exception.  Instead of re-throwing, return codes are used to communicate up the stack that a failure occurred.  `_teardown_logging()` is called whether or not the command class method succeeded, then the wrapper either succeeds or dies based on the return code value."},"comment":{"url":"https://api.github.com/repos/genome/genome/issues/comments/109162109","html_url":"https://github.com/genome/genome/pull/799#issuecomment-109162109","issue_url":"https://api.github.com/repos/genome/genome/issues/799","id":109162109,"user":{"login":"mkiwala","id":108881,"avatar_url":"https://avatars.githubusercontent.com/u/108881?v=3","gravatar_id":"","url":"https://api.github.com/users/mkiwala","html_url":"https://github.com/mkiwala","followers_url":"https://api.github.com/users/mkiwala/followers","following_url":"https://api.github.com/users/mkiwala/following{/other_user}","gists_url":"https://api.github.com/users/mkiwala/gists{/gist_id}","starred_url":"https://api.github.com/users/mkiwala/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mkiwala/subscriptions","organizations_url":"https://api.github.com/users/mkiwala/orgs","repos_url":"https://api.github.com/users/mkiwala/repos","events_url":"https://api.github.com/users/mkiwala/events{/privacy}","received_events_url":"https://api.github.com/users/mkiwala/received_events","type":"User","site_admin":false},"created_at":"2015-06-05T04:58:53Z","updated_at":"2015-06-05T04:58:53Z","body":"I don't like mixing return codes and exception catching paradigms either.  I don't consider this a mix of paradigms since all of the methods that yield return codes are designed to not throw exceptions.\r\n\r\nBefore creating this PR, I had considered the options you propose.  I like calling the cleanup method option better than single try/catch option.  Since we both find the first option acceptable, I'll close this PR, in favor of this alternative approach."}}