-
Notifications
You must be signed in to change notification settings - Fork 593
refactor(binding/python): Add multiple custom exception for each of error code in Rust Core #3492
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
Conversation
…rror code in Rust Core Signed-off-by: Manjusaka <[email protected]>
5242d1c
to
d95d518
Compare
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
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.
Mostly LGTM, @messense would you like to take another review?
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 think there should still be a catch-all opendal Exception just like the original Error
type, other exceptions should subclass from it, forming an exception hierarchy.
See also PyO3/pyo3#3452
So we will have |
Exactly, to catch any opendal exception, user should not need to catch A good exception hierarchy example is https://github.com/psf/requests/blob/main/src/requests/exceptions.py |
SGTM |
Signed-off-by: Manjusaka <[email protected]>
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.
Can we also add a test case to ensure that catching any opendal exception using Error
works?
Signed-off-by: Manjusaka <[email protected]>
This reverts commit 1d7d5a7.
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka [email protected]