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

Do not create conda error reports for common / expected exceptions #5264

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Mar 28, 2024

Description

Closes #5263

This is what we would need to close that issue without registering new base classes in conda exception handler.

CondaError calls super() so we can't compose new exception types that inherit both from CondaError and e.g. CalledProcessError. This prevents us from having a mixed exception that doesn't change which type of exception is raised (for the purpose of try/except idioms or isinstance checks).

I'd rather have this:

class BuildScriptException(CondaError, CalledProcessError):
  ...

and then call:

try:
  subprocess_for_build_script()
except CalledProcessError from exc:
  raise BuildScriptException(exc, ...) from exc

Achieving this ^ will require changes in conda/conda though (either changing the CondaError class so it doesn't call super(), or having a different base class for the purpose of exception handling like BaseCondaError).

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@jaimergp jaimergp requested a review from a team as a code owner March 28, 2024 14:23
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Mar 28, 2024
@jaimergp jaimergp changed the title Error gracefully Do not create conda error reports for common / expected exceptions Mar 28, 2024
Copy link

codspeed-hq bot commented Mar 28, 2024

CodSpeed Performance Report

Merging #5264 will not alter performance

Comparing jaimergp:error-gracefully (0f0fd8a) with main (aea5a22)

Summary

✅ 3 untouched benchmarks

@jaimergp
Copy link
Contributor Author

pre-commit.ci autofix

@jezdez
Copy link
Member

jezdez commented Apr 29, 2024

@kenodegard @jaimergp How does this overlap with #5255?

@jaimergp
Copy link
Contributor Author

Mostly compatible. We need to agree on a base exception for all conda-build exceptions, or just subclass CondaError for each one. I chose the former, but Ken's does:

class CondaBuildUserError(CondaError):
  ...

vs my idea:

class CondaBuildError(CondaError):
  ...

class CondaBuildUserError(CondaBuildError):
  ...

I don't mind either, but we need to agree on one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

Long conda info error messages for common exceptions
3 participants