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

Remove stderr print on SystemExit().code == None #143

Merged
merged 1 commit into from
Aug 11, 2015

Conversation

kwlzn
Copy link
Contributor

@kwlzn kwlzn commented Aug 6, 2015

The existing SystemExit handler causes an extraneous 'None' to be printed at exit when pex is used with twitter.common.app (and presumably sys.exit() is called with no arguments, which leads to SystemExit().code == None). This adds a None check on se.code.

@@ -321,7 +321,7 @@ def execute(self):
except SystemExit as se:
# Print a SystemExit error message, avoiding a traceback in python3.
# This must happen here, as sys.stderr is about to be torn down
if not isinstance(se.code, int):
if not isinstance(se.code, int) and se.code is not None:
print(se.code, file=sys.stderr)

Choose a reason for hiding this comment

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

This change LGTM.

However, w.r.t to previous change, we never want any exit code / message to be explicitly printed on stderr. Isn't it? I couldn't realize the need of it even after reading comment re: avoiding traceback in python3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

per the sys.exit docs:

'The optional argument arg can be an integer giving the exit status (defaulting to zero), or another type of object. If it is an integer, zero is considered “successful termination” and any nonzero value is considered “abnormal termination” by shells and the like. Most systems require it to be in the range 0-127, and produce undefined results otherwise. Some systems have a convention for assigning specific meanings to specific exit codes, but these are generally underdeveloped; Unix programs generally use 2 for command line syntax errors and 1 for all other kind of errors. If another type of object is passed, None is equivalent to passing zero, and any other object is printed to stderr and results in an exit code of 1. In particular, sys.exit("some error message") is a quick way to exit a program when an error occurs.'

@jamesbroadhead
Copy link
Contributor

lgtm - this does what we need without the spurious printing

Thanks (sorry)

@wickman
Copy link
Contributor

wickman commented Aug 10, 2015

@kwlzn could you rebase this PR? I can get it merged in and a fix pushed to unbreak pants

@kwlzn kwlzn force-pushed the kwlzn/sysexit_print_none branch from b1dc063 to 78fbb6b Compare August 11, 2015 03:49
@kwlzn
Copy link
Contributor Author

kwlzn commented Aug 11, 2015

@wickman ah my bad, should be clean now (and apologies for the delay, just saw this response). thx!

wickman added a commit that referenced this pull request Aug 11, 2015
Remove stderr print on SystemExit().code == None
@wickman wickman merged commit dda6501 into pex-tool:master Aug 11, 2015
@kwlzn kwlzn deleted the kwlzn/sysexit_print_none branch August 11, 2015 17:56
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.

4 participants