conda / conda-build

Commands and tools for building conda packages
https://docs.conda.io/projects/conda-build/
Other
383 stars 423 forks source link

Replace using sys.exit with higher level error/logic handling, e.g. custom exception #4209

Open beckermr opened 3 years ago

beckermr commented 3 years ago

What is the idea?

We here at conda-forge use some of the conda build APIs for our work. The code has been getting an increasing number of sys.exit calls for errors, which are causing our jobs to die.

Instead, maybe the code should raise an error and then catch it at the highest level and call sys.exit? This preserves the internals for others while maintaining the same behavior.

Why is this needed?

sys.exit calls are harder to catch/debug when using conda_build as an API.

What should happen?

Review and ideally remove all sys.exit calls.

Additional Context

These sys.exit calls in conda-build are very painful for downstream users of the code base as a library (i.e., conda-forge). The reason is that in the python exception hierarchy has the SystemExit exception raised by sys.exit and the standard Exception at the same level (https://docs.python.org/3/library/exceptions.html#exception-hierarchy). This means when one calls sys.exit, the invoking code cannot catch that exception without resorting to explicitly catching SystemExit as opposed to a normal Exception. The slight change in exception handling API breaks (at least my) expectations as a user of the code and has created quite a few mystery code crashes in the bot over the years.

Can we use a standard exception here and then have the conda-build CLI code handle that exception and call sys.exit?

_Originally posted by @beckermr in https://github.com/conda/conda-build/pull/5237#discussion_r1528736514_

### Tasks
- [ ] https://github.com/conda/conda-build/pull/5353
- [ ] https://github.com/conda/conda-build/pull/5255
- [ ] https://github.com/conda/conda-build/pull/5354
- [ ] https://github.com/conda/conda-build/pull/5367
beckermr commented 3 years ago

cc @mingwandroid for viz

jezdez commented 3 years ago

This was previously brought up in the community meeting and I just want to raise that this is a good idea and should instead be using a way that allows programmatic access to some of the internals. sys.exit is in my experience not a great way to handle runtime errors or even just code dead ends.

github-actions[bot] commented 1 year ago

Hi there, thank you for your contribution!

This issue has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this issue to remain open please:

  1. Verify that you can still reproduce the issue at hand
  2. Comment that the issue is still reproducible and include:
    • What OS and version you reproduced the issue on
    • What steps you followed to reproduce the issue

NOTE: If this issue was closed prematurely, please leave a comment.

Thanks!

beckermr commented 1 year ago

Not stale.