[GitHub] [arrow] lidavidm commented on a diff in pull request #12749: ARROW-16069: [C++][FlightRPC] Refactor out gRPC error code handling

[GitBox <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org