Github user chtyim commented on a diff in the pull request:
https://github.com/apache/twill/pull/40#discussion_r107767933
--- Diff:
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
public Map<String, Integer> getMaxRetries() {
return maxRetries;
}
+
+ /**
+ * Returns the ZK connection string for the Kafka used for log
collections,
+ * or {@code null} if log collection is disabled.
+ */
+ @Nullable
+ public String getKafkaZKConnect() {
+ if (!isLogCollectionEnabled()) {
--- End diff --
This is because there is no way to enforce someone have to call
`isLogCollectionEnabled` first before calling the `getKafkaZKConnect` method,
meaning the `getKafkaZKConnect` method needs to handle the case when log
collection disabled anyway (either by returning `null` or returning `Optional`
or throwing exception). From the caller perspective, it seems more straight
forward to call the method and validate against the contract then calling two
methods in the right order.
However, having `getKafkaZKConnect` returning `null` (I prefer `Optional`,
but since we are on Java 7, and we are on the road on reducing reliance on
guava, so that's not really an option), can make cases like this simpler in
future when adding support for TWILL-147. E.g.
```java
YarnTwillController(@Nullable String kafkaZKConnect, ...) {
this.kafkaClient = createKafkaClient(kafkaZKConnect);
}
@Nullable
private KafkaClient createKafkaClient(@Nnullable String kafkaZKConnect) {
if (kafkaZKConnect == null) {
return null;
}
return new KafkaClient(....);
}
...
// For app launch with the current twill runner
new YarnTwillController(spec.getKafkaZKConnect(), ...);
// For running app recovered from ZK.
new YarnTwillController(amLiveNode.getKafkaZKConnect(), ...);
```
Without the `nullable` support, there would be more overloaded constructors
as well as code branches to achieve the same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
