kou commented on code in PR #35378:
URL: https://github.com/apache/arrow/pull/35378#discussion_r1181462357


##########
cpp/src/arrow/flight/server_auth.h:
##########
@@ -55,8 +57,54 @@ class ARROW_FLIGHT_EXPORT ServerAuthHandler {
   virtual ~ServerAuthHandler();
   /// \brief Authenticate the client on initial connection. The server
   /// can send and read responses from the client at any time.
+  ///
+  /// If this version is implemented, the deprecated Authentication()
+  /// without ServerCallContext version isn't used. So we can
+  /// implement the deprecated version like the following:
+  ///
+  ///   Status Authenticate(ServerAuthSender* outgoing,
+  ///                       ServerAuthReader* incoming) override {
+  ///     return Status::NotImplemented("This version is never used");
+  ///   }
+  ///
+  /// \param[in] context The call context.
+  /// \param[in] outgoing The writer for messages to the client.
+  /// \param[in] incoming The reader for messages from the client.
+  /// \return Status OK if this authentication is succeeded.
+  virtual Status Authenticate(const ServerCallContext& context,
+                              ServerAuthSender* outgoing, ServerAuthReader* 
incoming) {
+    return Authenticate(outgoing, incoming);
+  }
+  /// \brief Authenticate the client on initial connection. The server
+  /// can send and read responses from the client at any time.
+  /// \param[in] outgoing The writer for messages to the client.
+  /// \param[in] incoming The reader for messages from the client.
+  /// \return Status OK if this authentication is succeeded.
+  /// \deprecated Deprecated since 13.0.0. Implement the Authentication()
+  /// with ServerCallContext version instead.
   virtual Status Authenticate(ServerAuthSender* outgoing, ServerAuthReader* 
incoming) = 0;

Review Comment:
   We can mark this as deprecated but client codes that overrides this don't 
receive any deprecated warning from their compiler. Because client codes don't 
call this method. Our server implementations call this method instead.
   
   Anyway, I've added `ARROW_DEPRECATED()` because a 
`-Wdocumentation-deprecated-sync` warning is reported without it: 
https://github.com/kou/arrow/actions/runs/4849400904/jobs/8641350382#step:9:1437
   
   ```text
   /Users/runner/work/arrow/arrow/cpp/src/arrow/flight/server_auth.h:83:8: 
error: declaration is marked with '\deprecated' command but does not have a 
deprecation attribute [-Werror,-Wdocumentation-deprecated-sync]
     /// \deprecated Deprecated since 13.0.0. Implement the Authentication()
         ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   /Users/runner/work/arrow/arrow/cpp/src/arrow/flight/server_auth.h:85:3: 
note: add a deprecation attribute to the declaration to silence this warning
     virtual Status Authenticate(ServerAuthSender* outgoing, ServerAuthReader* 
incoming) = 0;
     ^
   ```



-- 
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]

Reply via email to