You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/05/05 06:56:42 UTC
[GitHub] [arrow] kou opened a new issue, #35442: [C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()
kou opened a new issue, #35442:
URL: https://github.com/apache/arrow/issues/35442
### Describe the enhancement requested
I want to use `arrow::flight::ServerCallContext::peer()` to get client address in https://github.com/apache/arrow-flight-sql-postgresql because PostgreSQL can specify authentication configurations per client address.
See also: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html
Is it acceptable that we change `StartCall()` signature to
```cpp
virtual Status ServerMiddlewareFactory::StartCall(const CallInfo& info,
const ServerCallContext& context,
std::shared_ptr<ServerMiddleware>* middleware)
```
from
```cpp
virtual Status ServerMiddlewareFactory::StartCall(const CallInfo& info,
const CallHeaders& incoming_headers,
std::shared_ptr<ServerMiddleware>* middleware)
```
?
Here is an incomplete diff that illustrates this proposal:
```diff
diff --git a/cpp/src/arrow/flight/server_middleware.h b/cpp/src/arrow/flight/server_middleware.h
index 26431aff0..b8669a293 100644
--- a/cpp/src/arrow/flight/server_middleware.h
+++ b/cpp/src/arrow/flight/server_middleware.h
@@ -30,6 +30,8 @@
namespace arrow {
namespace flight {
+class ServerCallContext;
+
/// \brief Server-side middleware for a call, instantiated per RPC.
///
/// Middleware should be fast and must be infallible: there is no way
@@ -61,6 +63,28 @@ class ARROW_FLIGHT_EXPORT ServerMiddlewareFactory {
public:
virtual ~ServerMiddlewareFactory() = default;
+ /// \brief A callback for the start of a new call.
+ ///
+ /// Return a non-OK status to reject the call with the given status.
+ ///
+ /// \param[in] info Information about the call.
+ /// \param[in] context The call context.
+ /// \param[out] middleware The middleware instance for this call. If
+ /// null, no middleware will be added to this call instance from
+ /// this factory.
+ /// \return Status A non-OK status will reject the call with the
+ /// given status. Middleware previously in the chain will have
+ /// their CallCompleted callback called. Other middleware
+ /// factories will not be called.
+ virtual Status StartCall(const CallInfo& info, const ServerCallContext& context, ,
+ std::shared_ptr<ServerMiddleware>* middleware) {
+ // TODO: We can make this pure virtual function when we remove
+ // the deprecated version.
+ ARROW_SUPPRESS_DEPRECATION_WARNING
+ return StartCall(info, context.incoming_headers(), middleware);
+ ARROW_UNSUPPRESS_DEPRECATION_WARNING
+ }
+
/// \brief A callback for the start of a new call.
///
/// Return a non-OK status to reject the call with the given status.
@@ -75,8 +99,12 @@ class ARROW_FLIGHT_EXPORT ServerMiddlewareFactory {
/// given status. Middleware previously in the chain will have
/// their CallCompleted callback called. Other middleware
/// factories will not be called.
+ ARROW_DEPRECATED("Deprecated in 13.0.0. Use ServerCallContext overload instead.")
virtual Status StartCall(const CallInfo& info, const CallHeaders& incoming_headers,
- std::shared_ptr<ServerMiddleware>* middleware) = 0;
+ std::shared_ptr<ServerMiddleware>* middleware) {
+ return Status::NotImplemented(typeid(this).name(),
+ "::StartCall() isn't implemented");
+ }
};
} // namespace flight
diff --git a/cpp/src/arrow/flight/transport/grpc/grpc_server.cc b/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
index 09d702cd8..d0d618993 100644
--- a/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
+++ b/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
@@ -324,7 +324,7 @@ class GrpcServiceHandler final : public FlightService::Service {
for (const auto& factory : middleware_) {
std::shared_ptr<ServerMiddleware> instance;
Status result =
- factory.second->StartCall(info, flight_context.incoming_headers(), &instance);
+ factory.second->StartCall(info, flight_context, &instance);
if (!result.ok()) {
// Interceptor rejected call, end the request on all existing
// interceptors
```
### Component(s)
C++, FlightRPC
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on issue #35442: [C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #35442:
URL: https://github.com/apache/arrow/issues/35442#issuecomment-1536131675
I think this is reasonable.
--
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
[GitHub] [arrow] kou closed issue #35442: [C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou closed issue #35442: [C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()
URL: https://github.com/apache/arrow/issues/35442
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on issue #35442: [C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #35442:
URL: https://github.com/apache/arrow/issues/35442#issuecomment-1535825780
@lidavidm What do you think about this?
--
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