feat: impl ToStreamProst for stream exchange
#967
Conversation
rust/common/src/catalog/schema.rs
Outdated
There was a problem hiding this comment.
I'd prefer to return Result here.
There was a problem hiding this comment.
It's Ok. The problem is to_stream_prost_body is not return a Result. So there must a unwrap remain.
There was a problem hiding this comment.
I think we should refactor it to return a Result. I'll take this later. We can unwrap for now.
There was a problem hiding this comment.
Why? I think to_prost must succeed.
There was a problem hiding this comment.
EncodeError always indicates that a message failed to encode because the provided buffer had insufficient capacity. Message encoding is otherwise infallible.
You're right. It must succeed in our cases.
There was a problem hiding this comment.
But it's been changed to Result<Vec<ProstField>>😇
There was a problem hiding this comment.
Actually not unreachable, if it is Any or AnyShard, it will panic in above match arms. If it's Single/Broadcast, return a zero vector is expected.
Codecov Report
@@ Coverage Diff @@
## main #967 +/- ##
============================================
- Coverage 71.73% 71.69% -0.05%
Complexity 2766 2766
============================================
Files 971 972 +1
Lines 56825 56938 +113
Branches 1787 1787
============================================
+ Hits 40766 40820 +54
- Misses 15169 15228 +59
Partials 890 890
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
4655b13 to
0ae973b
Compare
What's changed and what's your intention?
Part of #967
Checklist
Refer to a related PR or issue link (optional)