-
Notifications
You must be signed in to change notification settings - Fork 425
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
Conversation
CodSpeed Performance ReportMerging #5264 will not alter performanceComparing Summary
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
@kenodegard @jaimergp How does this overlap with #5255? |
Mostly compatible. We need to agree on a base exception for all conda-build exceptions, or just subclass class CondaBuildUserError(CondaError):
... vs my idea: class CondaBuildError(CondaError):
...
class CondaBuildUserError(CondaBuildError):
... I don't mind either, but we need to agree on one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind either with how we define CondaBuildUserError
I only opted for CondaBuildUserError(CondaError)
to avoid any potential side-effects of inheriting directly from CondaBuildError
🤷🏼♂️
We've also decided it would be better to split #5255 into smaller PRs for easier review and testing so merging this ahead of reworking all of that would be good
@@ -1945,7 +1946,7 @@ def test_add_pip_as_python_dependency_from_condarc_file( | |||
testing_metadata, testing_workdir, add_pip_as_python_dependency, monkeypatch | |||
): | |||
""" | |||
Test whether settings from .condarc files are heeded. | |||
Test whether settings from .condarc files are needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test whether settings from .condarc files are needed. | |
Test whether settings from .condarc files are heeded. |
I think this wording edit needs to be reverted, since this test seems to be checking whether or not the settings in the .condarc
files are actually being taken into account/paid attention to (?)
Description
Closes #5263
This is what we would need to close that issue without registering new base classes in
conda
exception handler.CondaError
callssuper()
so we can't compose new exception types that inherit both fromCondaError
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 oftry/except
idioms orisinstance
checks).I'd rather have this:
and then call:
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 likeBaseCondaError
).Checklist - did you ...
news
directory (using the template) for the next release's release notes?