From 4d9ebcbd73fc4763302efa53c6b9a5fb9874197d Mon Sep 17 00:00:00 2001 From: David Li Date: Tue, 16 Mar 2021 13:28:53 -0400 Subject: [PATCH] ARROW-10250: [C++][FlightRPC] Consistently use FlightClientOptions::Defaults --- cpp/src/arrow/flight/client.cc | 5 +---- cpp/src/arrow/flight/client.h | 9 +++------ cpp/src/arrow/flight/flight_test.cc | 10 +++++----- python/pyarrow/_flight.pyx | 2 +- python/pyarrow/includes/libarrow_flight.pxd | 4 +++- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/cpp/src/arrow/flight/client.cc b/cpp/src/arrow/flight/client.cc index cd6f3a97e58..c0e8eaaed28 100644 --- a/cpp/src/arrow/flight/client.cc +++ b/cpp/src/arrow/flight/client.cc @@ -90,9 +90,6 @@ std::shared_ptr FlightWriteSizeStatusDetail::Unwrap return std::dynamic_pointer_cast(status.detail()); } -FlightClientOptions::FlightClientOptions() - : write_size_limit_bytes(0), disable_server_verification(false) {} - FlightClientOptions FlightClientOptions::Defaults() { return FlightClientOptions(); } struct ClientRpc { @@ -1240,7 +1237,7 @@ FlightClient::~FlightClient() {} Status FlightClient::Connect(const Location& location, std::unique_ptr* client) { - return Connect(location, {}, client); + return Connect(location, FlightClientOptions::Defaults(), client); } Status FlightClient::Connect(const Location& location, const FlightClientOptions& options, diff --git a/cpp/src/arrow/flight/client.h b/cpp/src/arrow/flight/client.h index 9d70067537a..b3c5a96e597 100644 --- a/cpp/src/arrow/flight/client.h +++ b/cpp/src/arrow/flight/client.h @@ -92,10 +92,7 @@ class ARROW_FLIGHT_EXPORT FlightWriteSizeStatusDetail : public arrow::StatusDeta int64_t actual_; }; -class ARROW_FLIGHT_EXPORT FlightClientOptions { - public: - FlightClientOptions(); - +struct ARROW_FLIGHT_EXPORT FlightClientOptions { /// \brief Root certificates to use for validating server /// certificates. std::string tls_root_certs; @@ -113,14 +110,14 @@ class ARROW_FLIGHT_EXPORT FlightClientOptions { /// Used to help limit server memory consumption. Only enabled if /// positive. When enabled, FlightStreamWriter.Write* may yield a /// IOError with error detail FlightWriteSizeStatusDetail. - int64_t write_size_limit_bytes; + int64_t write_size_limit_bytes = 0; /// \brief Generic connection options, passed to the underlying /// transport; interpretation is implementation-dependent. std::vector>> generic_options; /// \brief Use TLS without validating the server certificate. Use with caution. - bool disable_server_verification; + bool disable_server_verification = false; /// \brief Get default options. static FlightClientOptions Defaults(); diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc index 2cea4c490bb..099d416aae4 100644 --- a/cpp/src/arrow/flight/flight_test.cc +++ b/cpp/src/arrow/flight/flight_test.cc @@ -740,7 +740,7 @@ class TestTls : public ::testing::Test { } Status ConnectClient() { - auto options = FlightClientOptions(); + auto options = FlightClientOptions::Defaults(); CertKeyPair root_cert; RETURN_NOT_OK(ExampleTlsCertificateRoot(&root_cert)); options.tls_root_certs = root_cert.pem_cert; @@ -1891,7 +1891,7 @@ TEST_F(TestDoPut, DoPutSizeLimit) { const int64_t size_limit = 4096; Location location; ASSERT_OK(Location::ForGrpcTcp("localhost", server_->port(), &location)); - FlightClientOptions client_options; + auto client_options = FlightClientOptions::Defaults(); client_options.write_size_limit_bytes = size_limit; std::unique_ptr client; ASSERT_OK(FlightClient::Connect(location, client_options, &client)); @@ -2155,7 +2155,7 @@ TEST_F(TestTls, DoAction) { #if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS) TEST_F(TestTls, DisableServerVerification) { std::unique_ptr client; - auto client_options = FlightClientOptions(); + auto client_options = FlightClientOptions::Defaults(); // For security reasons, if encryption is being used, // the client should be configured to verify the server by default. ASSERT_EQ(client_options.disable_server_verification, false); @@ -2180,7 +2180,7 @@ TEST_F(TestTls, DisableServerVerification) { TEST_F(TestTls, OverrideHostname) { std::unique_ptr client; - auto client_options = FlightClientOptions(); + auto client_options = FlightClientOptions::Defaults(); client_options.override_hostname = "fakehostname"; CertKeyPair root_cert; ASSERT_OK(ExampleTlsCertificateRoot(&root_cert)); @@ -2199,7 +2199,7 @@ TEST_F(TestTls, OverrideHostname) { // Test the facility for setting generic transport options. TEST_F(TestTls, OverrideHostnameGeneric) { std::unique_ptr client; - auto client_options = FlightClientOptions(); + auto client_options = FlightClientOptions::Defaults(); client_options.generic_options.emplace_back(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, "fakehostname"); CertKeyPair root_cert; diff --git a/python/pyarrow/_flight.pyx b/python/pyarrow/_flight.pyx index 6bafcb99795..7a8dcdbbfa8 100644 --- a/python/pyarrow/_flight.pyx +++ b/python/pyarrow/_flight.pyx @@ -1055,7 +1055,7 @@ cdef class FlightClient(_Weakrefable): cdef: int c_port = 0 CLocation c_location = Location.unwrap(location) - CFlightClientOptions c_options + CFlightClientOptions c_options = CFlightClientOptions.Defaults() function[cb_client_middleware_start_call] start_call = \ &_client_middleware_start_call CIntStringVariant variant diff --git a/python/pyarrow/includes/libarrow_flight.pxd b/python/pyarrow/includes/libarrow_flight.pxd index cd052862200..161a8041c31 100644 --- a/python/pyarrow/includes/libarrow_flight.pxd +++ b/python/pyarrow/includes/libarrow_flight.pxd @@ -289,7 +289,6 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil: vector[pair[c_string, shared_ptr[CServerMiddlewareFactory]]] middleware cdef cppclass CFlightClientOptions" arrow::flight::FlightClientOptions": - CFlightClientOptions() c_string tls_root_certs c_string cert_chain c_string private_key @@ -299,6 +298,9 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil: vector[pair[c_string, CIntStringVariant]] generic_options c_bool disable_server_verification + @staticmethod + CFlightClientOptions Defaults() + cdef cppclass CFlightClient" arrow::flight::FlightClient": @staticmethod CStatus Connect(const CLocation& location,