Refactor exceptions to have a common base class (PP-991) #1695
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1695 +/- ##
=======================================
Coverage 90.21% 90.21%
=======================================
Files 245 245
Lines 28188 28199 +11
Branches 6447 6447
=======================================
+ Hits 25429 25441 +12
+ Misses 1823 1822 -1
Partials 936 936
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
core/exceptions.py
Outdated
| def __init__( | ||
| self, message: str | None = None, inner_exception: Exception | None = None | ||
| ): | ||
| def __init__(self, message: str | None = None): | ||
| """Initializes a new instance of BaseError class |
There was a problem hiding this comment.
BaseError -> BasePalaceException
| """Initializes a new instance of BaseError class | ||
|
|
||
| :param message: String containing description of the error occurred | ||
| :param inner_exception: (Optional) Inner exception |
There was a problem hiding this comment.
error -> exception that
And the inner_exception has gone away yes?
There was a problem hiding this comment.
Yes, inner_exception was removed. Its leftover from Python 2 days when there was no built in way to chain exceptions. Now that we are on Python 3 I replaced it with PEP3134 exception chaining in the couple places where inner_exception was previously used.
jonathangreen
force-pushed
the
feature/more-exception-refactoring
branch
from
98e672b to
2ae712e
Compare
|
@dbernstein I made the changes from mentioned in your code review comments. I can't merge though until you approve the PR review. |
Description
Rename
BaseErrorto beBasePalaceException. Refactor this class to remove the inner exception parameter, since Python 3 has exception chaining. Then refactor all the exceptions defined in our codebase to all have this as a base class, so its easier to catch all of our exceptions if we need to.Motivation and Context
Follow up to the work in #1694 which gave all our exceptions that return problem details a common base class.
How Has This Been Tested?
Checklist