Skip to content

CPP-966 Compilation warnings after cass_set_use_hostname_resolution() marked as deprecated #551

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
merged 5 commits into from
Jul 29, 2023
Merged

CPP-966 Compilation warnings after cass_set_use_hostname_resolution() marked as deprecated #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9924a9e
Change ccm updateconf logic to not enable MV and UDF in the DSE case.…
absurdfarce Jul 28, 2023
3ebea89
Remove the offending function from the (test) cluster building logic …
absurdfarce Jul 28, 2023
b18f40e
Remove beta protocol support from CLI options exposed via the integra…
absurdfarce Jul 28, 2023
010a3f7
Remove beta protocol support from test cluster object as well
absurdfarce Jul 28, 2023
663d39c
A drive-by fix: make the error messages for protocol version vs. serv…
absurdfarce Jul 28, 2023
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
2 changes: 1 addition & 1 deletion tests/src/integration/ccm/bridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ CCM::Bridge::generate_create_updateconf_command(CassVersion cassandra_version) {
updateconf_command.push_back("enable_scripted_user_defined_functions:true");
}

if (cassandra_version >= "4.0.0") {
if (cassandra_version >= "4.0.0" && !is_dse()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A small fix to get DSE working for integration tests. IIRC @weideng1 has something similar to this in another PR.

updateconf_command.push_back("enable_materialized_views:true");
updateconf_command.push_back("enable_user_defined_functions:true");
}
Expand Down
1 change: 0 additions & 1 deletion tests/src/integration/integration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ Integration::Integration()
, is_session_requested_(true)
, is_keyspace_change_requested_(true)
, is_test_chaotic_(false)
, is_beta_protocol_(Options::is_beta_protocol())
, protocol_version_(CASS_PROTOCOL_VERSION_V4)
, create_keyspace_query_("")
, start_time_(0ull) {
Expand Down
33 changes: 16 additions & 17 deletions tests/src/integration/integration.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,17 @@
return; \
}

#define SKIP_TEST_MESSAGE(server_version_string, comparison, version_string) \
#define SERVER_VERSION_SKIP_TEST_MESSAGE(server_version_string, comparison, version_string) \
"Unsupported for Apache Cassandra Version " << server_version_string << ": Server version is " \
<< comparison << " the specified version " << version_string

#define PROTOCOL_VERSION_SKIP_TEST_MESSAGE(server_version_string, comparison, version_string) \
"Unsupported for Apache Cassandra protocol version " << server_version_string << ": Server version is " \
<< comparison << " the specified protocol version " << version_string

/* Maintain existing behaviour; default message indicates server < specified */
#define SKIP_TEST_VERSION(server_version_string, version_string) \
SKIP_TEST(SKIP_TEST_MESSAGE(server_version_string, '<', version_string))
SKIP_TEST(SERVER_VERSION_SKIP_TEST_MESSAGE(server_version_string, '<', version_string))

#define CHECK_VERSION(version) \
do { \
Expand Down Expand Up @@ -114,25 +118,27 @@
if (!Options::is_cassandra()) { \
cass_version = static_cast<CCM::DseVersion>(cass_version).get_cass_version(); \
} \
std::vector<std::string> versions = Utils::explode(version_string,','); \
std::vector<std::string> versions = Utils::explode(version_string,','); \
for (unsigned int i = 0; i < versions.size(); i++) { \
CCM::CassVersion version = CCM::CassVersion(versions[i]); \
if (cass_version.major_version == version.major_version && \
cass_version.minor_version == version.minor_version && \
cass_version.patch_version >= version.patch_version) { \
SKIP_TEST( \
SKIP_TEST_MESSAGE( \
SERVER_VERSION_SKIP_TEST_MESSAGE( \
cass_version.to_string(), ">=", version.to_string())) \
} \
} \
} while (0)

#define CHECK_PROTOCOL_VERSION(version) \
do { \
int proto_version = this->protocol_version_; \
if (proto_version < version) { \
SKIP_TEST_VERSION(std::to_string(proto_version), #version) \
} \
#define CHECK_PROTOCOL_VERSION(version) \
do { \
int proto_version = this->protocol_version_; \
if (proto_version < version) { \
SKIP_TEST( \
PROTOCOL_VERSION_SKIP_TEST_MESSAGE( \
proto_version, '<', version)); \
} \
} while (0)

#define CHECK_OPTIONS_VERSION(version) \
Expand Down Expand Up @@ -343,13 +349,6 @@ class Integration : public testing::Test {
* destroyed
*/
bool is_test_chaotic_;
/**
* Flag to indicate if the beta protocol should be enabled. True if beta
* protocol should be enabled (Cassandra must be >= v3.10.0); false
* otherwise.
* (DEFAULT: true)
*/
bool is_beta_protocol_;
/**
* Workload to apply to the cluster
*/
Expand Down
29 changes: 0 additions & 29 deletions tests/src/integration/objects/cluster.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,6 @@ class Cluster : public Object<CassCluster, cass_cluster_free> {
return *this;
}

/**
* Use the newest beta protocol version
*
* @param enable True if beta protocol should be enable; false the highest
* non-beta protocol will be used (unless set) (default: false)
* @return Cluster object
*/
Cluster& with_beta_protocol(bool enable = false) {
EXPECT_EQ(CASS_OK, cass_cluster_set_use_beta_protocol_version(
get(), (enable == true ? cass_true : cass_false)));
return *this;
}

/**
* Sets the timeout for connecting to a node
*
Expand Down Expand Up @@ -206,22 +193,6 @@ class Cluster : public Object<CassCluster, cass_cluster_free> {
return *this;
}

/**
* Enable/Disable the use of hostname resolution
*
* This is useful for authentication (Kerberos) or encryption (SSL)
* services that require a valid hostname for verification.
*
* @param enable True if hostname resolution should be enabled; false
* otherwise (default: true)
* @return Cluster object
*/
Cluster& with_hostname_resolution(bool enable = true) {
EXPECT_EQ(CASS_OK, cass_cluster_set_use_hostname_resolution(
get(), (enable == true ? cass_true : cass_false)));
return *this;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The heart of the matter. The use of this function was the immediate cause of the slew of deprecated warnings when building the integration tests.

/**
* Sets the number of I/O threads. This is the number of threads that will
* handle query requests
Expand Down
10 changes: 0 additions & 10 deletions tests/src/integration/options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ std::string Options::public_key_ = "public.key";
std::string Options::private_key_ = "private.key";
bool Options::is_verbose_ccm_ = false;
bool Options::is_verbose_integration_ = false;
bool Options::is_beta_protocol_ = false;

// Static initialization is not guaranteed for the following types
CCM::DseCredentialsType Options::dse_credentials_type_;
Expand Down Expand Up @@ -185,8 +184,6 @@ bool Options::initialize(int argc, char* argv[]) {
is_verbose_ccm_ = true;
is_verbose_integration_ = true;
}
} else if (key == "--disable-beta-protocol") {
is_beta_protocol_ = false;
}
#ifdef CASS_USE_LIBSSH2
else if (key == "--authentication") {
Expand Down Expand Up @@ -380,11 +377,6 @@ void Options::print_help() {
std::cout << " --verbose(=ccm,integration)" << std::endl
<< " "
<< "Enable verbose output for component(s)." << std::endl;
std::cout << " --disable-beta-protocol" << std::endl
<< " "
<< "Disable beta protocol use by default." << std::endl
<< " "
<< "NOTE: Individual tests may override this setting." << std::endl;
std::cout << std::endl;
}

Expand Down Expand Up @@ -522,8 +514,6 @@ bool Options::is_verbose_ccm() { return is_verbose_ccm_; }

bool Options::is_verbose_integration() { return is_verbose_integration_; }

bool Options::is_beta_protocol() { return is_beta_protocol_; }

Options::Options() {}

bool Options::bool_value(const std::string& value) {
Expand Down
14 changes: 0 additions & 14 deletions tests/src/integration/options.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,6 @@ class Options {
* otherwise
*/
static bool is_verbose_integration();
/**
* Flag to determine if beta protocol should be enabled or not; should only
* pertain to the default setting.
*
* @return True if beta protocol should be enabled; false otherwise
*/
static bool is_beta_protocol();
/**
* Get a CCM instance based on the options
*
Expand Down Expand Up @@ -350,13 +343,6 @@ class Options {
* Flag to determine if verbose integration output should enabled
*/
static bool is_verbose_integration_;
/**
* Flag to determine if beta protocol should be enabled or not; should only
* pertain to the default setting.
*
* NOTE: Individual tests can still override this.
*/
static bool is_beta_protocol_;

/**
* Hidden default constructor
Expand Down
2 changes: 1 addition & 1 deletion tests/src/integration/tests/test_auth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class AuthenticationTests : public Integration {
// Configure and start the CCM cluster for plain text authentication usage
ccm_->update_cluster_configuration("authenticator", "PasswordAuthenticator");
ccm_->start_cluster("-Dcassandra.superuser_setup_delay_ms=0");
cluster_ = default_cluster().with_beta_protocol(false);
cluster_ = default_cluster();
}

protected:
Expand Down
2 changes: 0 additions & 2 deletions tests/src/integration/tests/test_dse_auth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ class AuthenticationTest : public DseIntegration {
Cluster cluster = dse::Cluster::build()
.with_gssapi_authenticator("dse", principal)
.with_contact_points(contact_points_)
.with_hostname_resolution(true)
.with_schema_metadata(false);
Session session = cluster.connect();

Expand All @@ -203,7 +202,6 @@ class AuthenticationTest : public DseIntegration {
Cluster cluster = dse::Cluster::build()
.with_plaintext_authenticator(username, password)
.with_contact_points(contact_points_)
.with_hostname_resolution(true)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These two usages are the only places where with_hostname_resolution() was actually used. Manually confirmed this test passes for all supported versions of DSE after this change.

.with_schema_metadata(false);
Session session = cluster.connect();

Expand Down
1 change: 0 additions & 1 deletion tests/src/integration/tests/test_exec_profile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class ExecutionProfileTest : public Integration {
, skip_base_execution_profile_(false) {
replication_factor_ = 2;
number_dc1_nodes_ = 2;
is_beta_protocol_ = false; // Issue with beta protocol v5 and functions on Cassandra v3.10.0+
}

void SetUp() {
Expand Down
5 changes: 2 additions & 3 deletions tests/src/integration/tests/test_prepared_metadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ CASSANDRA_INTEGRATION_TEST_F(PreparedMetadataTests, AlterDoesntUpdateColumnCount

// Ensure beta protocol is not set
Session session = default_cluster()
.with_beta_protocol(false)
.with_protocol_version(CASS_PROTOCOL_VERSION_V4)
.connect(keyspace_name_);

Expand All @@ -87,10 +86,10 @@ CASSANDRA_INTEGRATION_TEST_F(PreparedMetadataTests, AlterDoesntUpdateColumnCount
*/
CASSANDRA_INTEGRATION_TEST_F(PreparedMetadataTests, AlterProperlyUpdatesColumnCount) {
CHECK_FAILURE;
CHECK_VERSION(4.0.0);
CHECK_PROTOCOL_VERSION(CASS_PROTOCOL_VERSION_V5);

// Ensure protocol v5 or greater
Session session = default_cluster().with_beta_protocol(true).connect(keyspace_name_);
Session session = default_cluster().connect(keyspace_name_);

// The column count will properly update after the alter
prepared_check_column_count_after_alter(session, 3u);
Expand Down
3 changes: 0 additions & 3 deletions tests/src/integration/tests/test_set_keyspace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ CASSANDRA_INTEGRATION_TEST_F(SetKeyspaceTests, QueryNotSupported) {
CHECK_FAILURE;

Session session = default_cluster()
.with_beta_protocol(false)
.with_protocol_version(CASS_PROTOCOL_VERSION_V4)
.connect();

Expand Down Expand Up @@ -216,7 +215,6 @@ CASSANDRA_INTEGRATION_TEST_F(SetKeyspaceTests, PreparedNotSupported) {
CHECK_FAILURE;

Session session = default_cluster()
.with_beta_protocol(false)
.with_protocol_version(CASS_PROTOCOL_VERSION_V4)
.connect();

Expand Down Expand Up @@ -362,7 +360,6 @@ CASSANDRA_INTEGRATION_TEST_F(SetKeyspaceTests, BatchNotSupported) {
CHECK_FAILURE;

Session session = default_cluster()
.with_beta_protocol(false)
.with_protocol_version(CASS_PROTOCOL_VERSION_V4)
.connect();

Expand Down