Copilot commented on code in PR #49662:
URL: https://github.com/apache/arrow/pull/49662#discussion_r3036909600
##########
cpp/src/arrow/flight/transport/grpc/grpc_client.cc:
##########
@@ -732,19 +732,31 @@ class GrpcClientImpl : public internal::ClientTransport {
# endif // defined(GRPC_USE_CERTIFICATE_VERIFIER)
# if defined(GRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS)
+# if GRPC_CPP_VERSION_CHECK(1, 80, 0)
+ auto certificate_provider =
+
std::make_shared<::grpc::experimental::InMemoryCertificateProvider>();
+
RETURN_NOT_OK(FromAbslStatus(certificate_provider->UpdateRoot(kDummyRootCert)));
+# else
auto certificate_provider =
std::make_shared<::grpc::experimental::StaticDataCertificateProvider>(
kDummyRootCert);
Review Comment:
PR description says to use
`grpc::experimental::InMemoryDataCertificateProvider`, but the code uses
`::grpc::experimental::InMemoryCertificateProvider`. Please align the PR
description with the actual type used (or update the code if the intended type
is different).
##########
cpp/src/arrow/flight/transport/grpc/util_internal.cc:
##########
@@ -331,6 +331,49 @@ ::grpc::Status ToGrpcStatus(const Status& arrow_status,
::grpc::ServerContext* c
return status;
}
+#if GRPC_CPP_VERSION_CHECK(1, 80, 0)
+Status FromAbslStatus(const ::absl::Status& absl_status) {
+ switch (absl_status.code()) {
+ case ::absl::StatusCode::kOk:
+ return Status::OK();
+ case ::absl::StatusCode::kCancelled:
+ return Status::Cancelled(absl_status.ToString());
+ case ::absl::StatusCode::kUnknown:
+ return Status::UnknownError(absl_status.ToString());
+ case ::absl::StatusCode::kInvalidArgument:
+ return Status::Invalid(absl_status.ToString());
+ case ::absl::StatusCode::kDeadlineExceeded:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kNotFound:
+ return Status::KeyError(absl_status.ToString());
+ case ::absl::StatusCode::kAlreadyExists:
+ return Status::AlreadyExists(absl_status.ToString());
+ case ::absl::StatusCode::kPermissionDenied:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kResourceExhausted:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kFailedPrecondition:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kAborted:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kOutOfRange:
+ return Status::Invalid(absl_status.ToString());
+ case ::absl::StatusCode::kUnimplemented:
+ return Status::NotImplemented(absl_status.ToString());
+ case ::absl::StatusCode::kInternal:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kUnavailable:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kDataLoss:
+ return Status::IOError(absl_status.ToString());
+ case ::absl::StatusCode::kUnauthenticated:
+ return Status::IOError(absl_status.ToString());
+ default:
+ return Status::UnknownError(absl_status.ToString());
+ }
Review Comment:
This file uses `::absl::Status` / `::absl::StatusCode` in `FromAbslStatus`
but doesn’t directly include the Abseil status header. Please include the
defining header here as well (rather than relying on gRPC’s transitive
includes) to make the build more robust across gRPC versions/packaging.
##########
cpp/src/arrow/flight/transport/grpc/util_internal.h:
##########
@@ -90,6 +96,12 @@ ARROW_FLIGHT_EXPORT
::grpc::Status ToGrpcStatus(const Status& arrow_status,
::grpc::ServerContext* ctx = nullptr);
+// gRPC 1.80.0 or later use absl::Status.
+#if GRPC_CPP_VERSION_CHECK(1, 80, 0)
+/// Convert an Abseil status to an Arrow status.
+ARROW_FLIGHT_EXPORT
+Status FromAbslStatus(const ::absl::Status& absl_status);
+#endif
Review Comment:
`FromAbslStatus` is declared with `::absl::Status`, but this header doesn’t
include an Abseil status header or forward-declare `absl::Status`. To avoid
relying on transitive includes, please add an appropriate include (e.g., Abseil
status header) or a forward declaration for `absl::Status` guarded by the same
version check.
--
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]
