celery / billiard

Multiprocessing Pool Extensions
Other
419 stars 252 forks source link

Pass arguments when wrapping sys.exit #275

Closed epruesse closed 5 years ago

epruesse commented 5 years ago

The sys.exit() wrapping in pool.Worker.__call__ does not pass status to sys.exit(arg). This is an issue with 3rd party libs that use sys.exit(message) for fatal conditions.

E.g.:

def in_third_party():
   sys.exit("I cannot go on")

@celery.task
def do_third_party_call():
  try:
    in_third_party()
  except SystemExit as exc:
    logger.error(str(exc))
    handle_error_message(str(exc))

This will log "I cannot go on" and pass it for handling as long as we are not inside of a celery worker. Inside of a worker, it will log "".

It might be arguable whether using sys.exit() in such a way is good design, but since this use is even suggested by the Python documentation, and since there is often no choice or control over 3rd party libs, it's something to live with.

The patch is intentionally minimal. I'm not certain exactly what's going on at that point in the code, and "fork" points are always finicky across platforms. So while I don't quite understand why sys.exit() needs to be patched in the first place, I've just passed the parameter on so it can be caught by tasks.

epruesse commented 5 years ago

Looks like Python 2.7 on Windows does not like this for whatever reason.

epruesse commented 5 years ago

I am assuming that the error on win32/py2k is normal then, or rather, caused by something other than my patch?

auvipy commented 5 years ago

ignore the winbuild

epruesse commented 5 years ago

In that case the if sys.platform can go. Not sure if it's necessary, triggers only in the jobs that fail anyway. Do delete it or do I create another PR?

auvipy commented 5 years ago

you can come with another pr

epruesse commented 5 years ago

See #276