Skip to content

GH-49661: [CI][C++] Suppress deprecated warnings with gRPC 1.80.0 #49662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kou merged 1 commit into from
Apr 6, 2026
+70 −3
Merged

GH-49661: [CI][C++] Suppress deprecated warnings with gRPC 1.80.0 #49662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions cpp/src/arrow/flight/transport/grpc/grpc_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +735 to +739
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code calls FromAbslStatus(...), but FromAbslStatus is only declared when ABSL_NAMESPACE_BEGIN is defined. Please either (1) ensure Abseil status headers are included so the symbol is always available in this build configuration, or (2) guard this call with the same preprocessor condition to avoid compilation failures in builds where Abseil isn’t present/used.

Copilot uses AI. Check for mistakes.
auto certificate_provider =
std::make_shared<::grpc::experimental::StaticDataCertificateProvider>(
kDummyRootCert);
Comment on lines +735 to 742
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
# endif
# if defined(GRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS)
::grpc::experimental::TlsChannelCredentialsOptions tls_options(
certificate_provider);
# else // defined(GRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS)
// While gRPC >= 1.36 does not require a root cert (it has a default)
// in practice the path it hardcodes is broken. See grpc/grpc#21655.
# else // defined(GRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS)
// While gRPC >= 1.36 does not require a root cert (it has a default)
// in practice the path it hardcodes is broken. See grpc/grpc#21655.
::grpc::experimental::TlsChannelCredentialsOptions tls_options;
# if GRPC_CPP_VERSION_CHECK(1, 80, 0)
tls_options.set_root_certificate_provider(certificate_provider);
# else
tls_options.set_certificate_provider(certificate_provider);
# endif
# endif // defined(GRPC_USE_TLS_CHANNEL_CREDENTIALS_OPTIONS_ROOT_CERTS)
# if !GRPC_CPP_VERSION_CHECK(1, 80, 0)
tls_options.watch_root_certs();
# endif
tls_options.set_root_cert_name("dummy");
# if defined(GRPC_USE_CERTIFICATE_VERIFIER)
tls_options.set_certificate_verifier(std::move(cert_verifier));
Expand Down
43 changes: 43 additions & 0 deletions cpp/src/arrow/flight/transport/grpc/util_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Comment on lines +334 to +373
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}
#endif

} // namespace grpc
} // namespace transport
} // namespace flight
Expand Down
12 changes: 12 additions & 0 deletions cpp/src/arrow/flight/transport/grpc/util_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ class Status;

namespace flight {

#define GRPC_CPP_VERSION_CHECK(major, minor, patch) \
((GRPC_CPP_VERSION_MAJOR > (major) || \
(GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR > (minor)) || \
((GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR == (minor) && \
GRPC_CPP_VERSION_PATCH >= (patch)))))
Comment on lines +37 to +41
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GRPC_CPP_VERSION_CHECK relies on GRPC_CPP_VERSION_{MAJOR,MINOR,PATCH} but this header doesn’t include the header that defines them. Please include the appropriate gRPC version header (e.g. grpcpp/version_info.h) or add a defensive #ifdef GRPC_CPP_VERSION_MAJOR fallback to avoid brittle transitive-include dependencies.

Copilot uses AI. Check for mistakes.

#define GRPC_RETURN_NOT_OK(expr) \
do { \
::arrow::Status _s = (expr); \
Expand Down Expand Up @@ -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
Comment on lines +99 to +104
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
} // namespace grpc
} // namespace transport
} // namespace flight
Expand Down
Loading