Include support for comma seperated strings for basic.contact-points

[utkuaydn]

The motivation for the PR is that in my project while using environment variables for the configuration via a Helm chart, I am unable to pass an array as an environment variable because it ends up as a string in the application configuration. Please let me know if I missed anything in the PR. I have already signed the CLA.

[utkuaydn]

This has been sitting here for a while, can anyone take a look? @absurdfarce

[aratno]

Could you add a unit test? I'd go about this by extracting the contact points behavior to a method in SessionBuilder, and testing that. There are also quite a few failing unit tests, so run mvn clean verify locally to see those. As a project, we're working on getting CI moved over to the new repo under ASF.

In terms of behavior - I'd prefer if we permitted lists but expanded comma-separated entries, so providing a list like this:

["a,b", "c"]

yielded contact points:

["a", "b", "c"]

The current implementation of the patch could change the interpretation of lists for existing users, which we shouldn't break. But individual hostnames shouldn't include commas, so would be safe to split those.

[aratno]

Suggested change

	# This must be a list of strings or a comma seperated string with each contact point specified as "host:port".
	# This must be a list of strings or a comma separated string with each contact point specified as "host:port".

[aratno]

I'm revising my review here - I think it would be confusing for users if some list-based configs supported comma-delimited values, and others didn't. You can set contact points using the programmatic config loader like this:

List<String> contactPoints = new ArrayList();
contactPoints.add("host1");
DriverConfigLoader configLoader = new DefaultProgrammaticDriverConfigLoaderBuilder()
        .withStringList(DefaultDriverOption.CONTACT_POINTS, contactPoints)
        .build();
try (CqlSession session = CqlSession.builder().withConfigLoader(configLoader).build()) {
    // ...
}

I think we should encourage use of the programmatic builder in cases like your example, rather than creating new syntax for them.

[utkuaydn]

I understand your point of view but the fact is that the approach you are suggesting is unusable with environment variables and configmaps from a HOCON point of view. Moreover, in our case, we are not using the library directly, but rather a Pekko library that uses this driver. So even if we could have one environment variable for multiple contact points, we cannot build a CqlSession and pass it along as an underlying session to the Pekko connector. Also, about the first review you posted, I don't think having ["a,b", "c"] makes much sense because I doubt that users will have comma-separated items and other single items in the list at the same time.

[aratno]

Is there a reason you can't configure Pekko to use a custom session? It looks like that should be possible from this documentation: https://pekko.apache.org/docs/pekko-connectors/current/cassandra.html#custom-session-creation

You can create a custom CqlSessionProvider that sets contact points based on the environment: https://pekko.apache.org/api/pekko-connectors/snapshot/org/apache/pekko/stream/connectors/cassandra/CqlSessionProvider.html

The main drawbacks of the implementation in this PR are:

1. Changing the parsing behavior of basic.contact-points could break existing client configurations
2. Allowing some list-typed configurations to support comma-separated strings when others don't is confusing for users