feat(frontend): introduce scheduler error #3156
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has totally checked 842 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 840 | 1 | 1 | 0 |
Click to see the invalid file list
- src/frontend/src/scheduler/error.rs
lmatz
force-pushed
the
lz/error
branch
2 times, most recently
from
caf0cd1
to
792f629
Compare
Codecov Report
@@ Coverage Diff @@
## main #3156 +/- ##
=======================================
Coverage 73.66% 73.66%
=======================================
Files 739 739
Lines 101792 101792
=======================================
Hits 74986 74986
Misses 26806 26806
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 |
| @@ -176,7 +181,7 @@ impl QueryResultFetcher { | |||
| .await?; | |||
| let mut stream = compute_client.get_data(self.task_output_id.clone()).await?; | |||
| while let Some(response) = stream.next().await { | |||
| yield DataChunk::from_protobuf(response.to_rw_result()?.get_record_batch()?)? | |||
| yield DataChunk::from_protobuf(response.map_err(TonicStatus)?.get_record_batch()?)?; | |||
There was a problem hiding this comment.
We may also use an RpcError variant for this and implement From<..> for this so there's no need to map_err here.
There was a problem hiding this comment.
yield returns RwError, tonic::Status -> SchedulerError::RpcError -> RwError, at least one map is needed I guess?
lmatz
force-pushed
the
lz/error
branch
2 times, most recently
from
4891da6
to
2925733
Compare
|
Should we introduct a |
lmatz
force-pushed
the
lz/error
branch
2 times, most recently
from
96c7f9a
to
cf4746f
Compare
In the long run, I believe it is very likely to do so. Scheduling is quite basic right now. |
What's changed and what's your intention?
As per the title.
Checklist
- [ ] I have written necessary docs and comments- [ ] I have added necessary unit tests and integration tests./risedev check(or alias,./risedev c)Refer to a related PR or issue link (optional)
#2366