lidavidm commented on code in PR #12749:
URL: https://github.com/apache/arrow/pull/12749#discussion_r849427961
##########
cpp/src/arrow/flight/transport.cc:
##########
@@ -159,6 +160,198 @@ TransportRegistry* GetDefaultTransportRegistry() {
return &kRegistry;
}
+//------------------------------------------------------------
+// Error propagation helpers
+
+TransportStatus TransportStatus::FromStatus(const Status& arrow_status) {
+ if (arrow_status.ok()) {
+ return TransportStatus{TransportStatusCode::kOk, ""};
+ }
+
+ TransportStatusCode code = TransportStatusCode::kUnknown;
+ std::string message = arrow_status.message();
+ if (arrow_status.detail()) {
+ message += ". Detail: ";
+ message += arrow_status.detail()->ToString();
+ }
+
+ std::shared_ptr<FlightStatusDetail> flight_status =
+ FlightStatusDetail::UnwrapStatus(arrow_status);
+ if (flight_status) {
+ switch (flight_status->code()) {
+ case FlightStatusCode::Internal:
+ code = TransportStatusCode::kInternal;
+ break;
Review Comment:
It's mostly an artifact of code history…we have this table in the spec:
https://arrow.apache.org/docs/format/Flight.html#error-handling and not all
codes mapped cleanly to an existing C++ status so a mix of Status and
FlightStatusDetail is used. But for transports, I wanted to force them to map
to and from the "abstract" error codes instead of ad-hoc mapping to Arrow/gRPC
statuses, hence TransportStatus. Those do feel rather redundant.
I could probably rework this to just work with Status/FlightStatusDetail
then, and accept some messiness when re-mapping it to/from gRPC/UCX?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
