Page MenuHomePhabricator

Gracefully handle SystemExit in user code
Needs RevisionPublic

Authored by max on Jun 16 2020, 12:05 AM.

Details

Summary

Sophisticated users can still bail with sys._exit -- this is to deal with, e.g., library code where unsophisticated users are using sys.exit for control flow.

Test Plan

Unit

Diff Detail

Repository
R1 dagster
Branch
system-exit
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

max created this revision.Jun 16 2020, 12:05 AM
max requested review of this revision.Jun 16 2020, 12:18 AM
schrockn requested changes to this revision.Jun 16 2020, 1:31 PM

Are we sure this is desirable behavior? Seems like we should honor SystemExit and exit the process...

This revision now requires changes to proceed.Jun 16 2020, 1:31 PM

I like this since:

  • we do cleanup and correct reporting
  • in process and multiprocess executors would now have the same response behavior

I guess we end up reraising so it is fine? Seems like maybe we should reraise the SystemExit error in this case?