sjvanrossum commented on code in PR #39284:
URL: https://github.com/apache/beam/pull/39284#discussion_r3561494828
##########
sdks/java/testing/kafka-service/src/test/java/org/apache/beam/sdk/testing/kafka/LocalKafka.java:
##########
@@ -20,18 +20,23 @@
import java.nio.file.Files;
import java.util.Properties;
import kafka.server.KafkaConfig;
-import kafka.server.KafkaServerStartable;
+import kafka.server.KafkaServer;
public class LocalKafka {
- private final KafkaServerStartable server;
+ private final KafkaServer server;
LocalKafka(int kafkaPort, int zookeeperPort) throws Exception {
Properties kafkaProperties = new Properties();
kafkaProperties.setProperty("port", String.valueOf(kafkaPort));
kafkaProperties.setProperty("zookeeper.connect",
String.format("localhost:%s", zookeeperPort));
kafkaProperties.setProperty("offsets.topic.replication.factor", "1");
kafkaProperties.setProperty("log.dir",
Files.createTempDirectory("kafka-log-").toString());
- server = new KafkaServerStartable(KafkaConfig.fromProps(kafkaProperties));
+ server =
+ new KafkaServer(
+ KafkaConfig.fromProps(kafkaProperties),
+ KafkaServer.$lessinit$greater$default$2(),
+ KafkaServer.$lessinit$greater$default$3(),
+ KafkaServer.$lessinit$greater$default$4());
Review Comment:
> Using Scala compiler-generated default argument methods (like
$lessinit$greater$default$2()) directly in Java is fragile and highly
discouraged.
Needs citation?
Jokes aside, that last argument is supposed to be a `Boolean` so this is not
exactly the right suggestion.
I don't feel strongly about this, Kafka itself instantiates `KafkaServer` as
follows in main:
```scala
new KafkaServer(
config,
Time.SYSTEM,
threadNamePrefix = None,
enableForwarding = enableApiForwarding(config)
)
```
Other than readability I don't see a good reason not to use the generated
methods to access default arguments.
If the signature were to change due to argument reordering as mentioned,
then the suggested change would break at compile time whereas the generated
methods would return the expected type because their definition depends on the
signature definition itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
