Not annotated with @VisibleForTesting since the property is not accessed directly in tests, but neither are the properties which do specify @VisibleForTesting.

Any chance this is null at this point? In the split below you have null checks, but not here.

Is this referring to topics? Both topicPartitions and topics are initialized as empty lists in the builder by default and replaced using .withTopics() and .withTopicPartitions(). The previous Preconditions.checkStateNotNull(topics) expression in the for loop should still not be null under any circumstance. Special care should be taken to carry that property forward when we add support for this property in KafkaIO's ExternalTransformRegistrar though, since it doesn't guarantee the same object state the builder guarantees.
In regards to topicPattern, if both topicPartitions and topics are empty, then topicPattern must be non-null, since the PTransform's expansion checks that at least one of those properties is set and the .withX() builder methods check that none are previously set.

As far as Kafka's topic metadata goes, .partitionsFor() will throw an exception if an unauthorized topic is requested and .listTopics() will only list all authorized topics. Both methods return initialized objects and ensure that potential null responses from the server are translated to empty collections (as far back as org.apache.kafka:kafka-clients:0.11.0.3) or throw an exception in the case of an authorization failure. I'd say that the existing check on partitionInfoList seems superfluous and could potentially be considered for deletion:

List<PartitionInfo> partitionInfoList = consumer.partitionsFor(topic);
checkState(
    partitionInfoList != null,
    "Could not find any partitions info. Please check Kafka configuration and make sure "
        + "that provided topics exist.");
for (PartitionInfo p : partitionInfoList) {
  partitions.add(new TopicPartition(p.topic(), p.partition()));
}

can you add a test, or update this test, such that you don't match all the topics?

Added one for partial matches and one for no matches.

Can change this API to use a portable type (for example, a string regex) so that this can be supported via cross-language wrappers ?

Read.External.Configuration has fields for keyDeserializer and valueDeserializer which are resolved from String to Class during external transform construction, does it make sense to provide a mapping there instead?

Ideally we don't want to be using external read configuration as a pattern. It makes it so the configurations for an IO diverge for python v. java users

Got it. Would an additional method overload with String topicPattern help at all?
Otherwise I'll change the method signature.

I think we can have both

Is there a significant advantage to using "Pattern" over a string regex ?

If it's a perf issue, we could just build the Patter object once within "withTopicPattern" and use that ?

Mostly to make sure that pattern flags can be specified by users, but nearly all (except LITERAL and CANON_EQ) can be specified in the expression as well.

Thanks. If there is no significant advantage for specifying a Pattern object I would just support specifying a String regex to keep the API simple.

Done.