lidavidm commented on code in PR #36205:
URL: https://github.com/apache/arrow/pull/36205#discussion_r1242178102
##########
cpp/src/arrow/flight/transport.h:
##########
@@ -223,44 +236,30 @@ ARROW_FLIGHT_EXPORT
TransportRegistry* GetDefaultTransportRegistry();
//------------------------------------------------------------
-// Error propagation helpers
-
-/// \brief Abstract status code as per the Flight specification.
-enum class TransportStatusCode {
- kOk = 0,
- kUnknown = 1,
- kInternal = 2,
- kInvalidArgument = 3,
- kTimedOut = 4,
- kNotFound = 5,
- kAlreadyExists = 6,
- kCancelled = 7,
- kUnauthenticated = 8,
- kUnauthorized = 9,
- kUnimplemented = 10,
- kUnavailable = 11,
-};
+// Async APIs
-/// \brief Abstract error status.
+/// \brief Transport-specific state for an async RPC.
///
-/// Transport implementations may use side channels (e.g. HTTP
-/// trailers) to convey additional information to reconstruct the
-/// original C++ status for implementations that can use it.
-struct ARROW_FLIGHT_EXPORT TransportStatus {
- TransportStatusCode code;
- std::string message;
-
- /// \brief Convert a C++ status to an abstract transport status.
- static TransportStatus FromStatus(const Status& arrow_status);
-
- /// \brief Reconstruct a string-encoded TransportStatus.
- static TransportStatus FromCodeStringAndMessage(const std::string& code_str,
- std::string message);
+/// Transport implementations may subclass this to store their own
+/// state, and stash an instance in a user-supplied AsyncListener via
+/// ClientTransport::GetAsyncRpc and ClientTransport::SetAsyncRpc.
+class AsyncRpc {
+ public:
+ virtual ~AsyncRpc() = default;
+ /// \brief Request cancellation of the RPC.
+ virtual void TryCancel() {}
- /// \brief Convert an abstract transport status to a C++ status.
- Status ToStatus() const;
+ /// Only needed for DoPut/DoExchange
+ virtual void Begin(std::shared_ptr<Schema> schema, ipc::IpcWriteOptions
options) {}
+ /// Only needed for DoPut/DoExchange
+ virtual void Write(arrow::flight::FlightStreamChunk chunk, bool last) {}
+ /// Only needed for DoPut/DoExchange
+ virtual void DoneWriting() {}
Review Comment:
No, I'm actually removing this (this is a slight optimization that gRPC
implements but I don't think it's worth exposing here)
##########
cpp/src/arrow/flight/transport.h:
##########
@@ -223,44 +236,30 @@ ARROW_FLIGHT_EXPORT
TransportRegistry* GetDefaultTransportRegistry();
//------------------------------------------------------------
-// Error propagation helpers
-
-/// \brief Abstract status code as per the Flight specification.
-enum class TransportStatusCode {
- kOk = 0,
- kUnknown = 1,
- kInternal = 2,
- kInvalidArgument = 3,
- kTimedOut = 4,
- kNotFound = 5,
- kAlreadyExists = 6,
- kCancelled = 7,
- kUnauthenticated = 8,
- kUnauthorized = 9,
- kUnimplemented = 10,
- kUnavailable = 11,
-};
+// Async APIs
-/// \brief Abstract error status.
+/// \brief Transport-specific state for an async RPC.
///
-/// Transport implementations may use side channels (e.g. HTTP
-/// trailers) to convey additional information to reconstruct the
-/// original C++ status for implementations that can use it.
-struct ARROW_FLIGHT_EXPORT TransportStatus {
- TransportStatusCode code;
- std::string message;
-
- /// \brief Convert a C++ status to an abstract transport status.
- static TransportStatus FromStatus(const Status& arrow_status);
-
- /// \brief Reconstruct a string-encoded TransportStatus.
- static TransportStatus FromCodeStringAndMessage(const std::string& code_str,
- std::string message);
+/// Transport implementations may subclass this to store their own
+/// state, and stash an instance in a user-supplied AsyncListener via
+/// ClientTransport::GetAsyncRpc and ClientTransport::SetAsyncRpc.
+class AsyncRpc {
+ public:
+ virtual ~AsyncRpc() = default;
+ /// \brief Request cancellation of the RPC.
+ virtual void TryCancel() {}
- /// \brief Convert an abstract transport status to a C++ status.
- Status ToStatus() const;
+ /// Only needed for DoPut/DoExchange
+ virtual void Begin(std::shared_ptr<Schema> schema, ipc::IpcWriteOptions
options) {}
+ /// Only needed for DoPut/DoExchange
+ virtual void Write(arrow::flight::FlightStreamChunk chunk, bool last) {}
+ /// Only needed for DoPut/DoExchange
+ virtual void DoneWriting() {}
Review Comment:
this being `last`, that is
--
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]
