ARROW-7312: [Rust] Implement std::error::Error for ArrowError. #5959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@andygrove @nevi-me @sunchao @paddyhoran Please help to review. |
nevi-me
reviewed
Dec 4, 2019
rust/arrow/src/error.rs
Outdated
| @@ -70,4 +71,12 @@ impl From<::std::string::FromUtf8Error> for ArrowError { | |||
| } | |||
| } | |||
|
|
|||
| impl Display for ArrowError { | |||
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | |||
| write!(f, "Arrow error happened!") | |||
There was a problem hiding this comment.
Why not display the type of and details ArrowError?
There was a problem hiding this comment.
Details should be displayed in debug mode.
There was a problem hiding this comment.
I still don't understand, but perhaps it's because I'm not familiar with the Display trait.
There was a problem hiding this comment.
Yea, I'm with @nevi-me here. His point is that "Arrow error happened!" is not that informative. You would also want to know the variant of the enum and any payload, right?
nevi-me
approved these changes
Dec 5, 2019
paddyhoran
approved these changes
Dec 6, 2019
There was a problem hiding this comment.
LGTM, thanks @liurenjie1024. I don't see CI failing but I guess we should wait before merging.
|
AppVeyor no longer runs any Rust tests, maybe we should remove it from the Rust runs (I haven't checked how it gets triggered though, so maybe that's not possible). Is it possible @kou? |
|
Rust job on AppVeyor has been migrated to GitHub Actions based job. So we don't need to wait for AppVeyor result for changes that only affects to Rust. FYI: We can disable AppVeyor job check for Rust only changes entirely by the following change: diff --git a/appveyor.yml b/appveyor.yml
index a6d79de7e..cd8617893 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -31,7 +31,6 @@ only_commits:
- python/
- r/
- ruby/
- - rust/
cache:
- C:\Users\Appveyor\clcache1 |
nevi-me
pushed a commit
to nevi-me/arrow
that referenced
this pull request
Dec 7, 2019
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
nevi-me
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Jan 1, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
nevi-me
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Jan 8, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
andy-thomason
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Jan 9, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
andy-thomason
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Jan 28, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
kszucs
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Feb 7, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
andy-thomason
pushed a commit
to andy-thomason/arrow
that referenced
this pull request
Feb 24, 2020
Add this implementation so that others can handle errors from arrow crate better. Closes apache#5959 from liurenjie1024/arrow-7312 and squashes the following commits: f27683a <Renjie Liu> Fix comment d3e238d <Renjie Liu> Implement error Authored-by: Renjie Liu <[email protected]> Signed-off-by: Neville Dipale <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add this implementation so that others can handle errors from arrow crate better.