[
https://issues.apache.org/jira/browse/TWILL-122?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15939116#comment-15939116
]
ASF GitHub Bot commented on TWILL-122:
--------------------------------------
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.
> Allow disabling the log transport
> ---------------------------------
>
> Key: TWILL-122
> URL: https://issues.apache.org/jira/browse/TWILL-122
> Project: Apache Twill
> Issue Type: Improvement
> Components: core
> Reporter: Terence Yim
> Assignee: Terence Yim
> Fix For: 0.11.0
>
>
> Currently transporting logs to Kafka is mandatory. It should be optionally so
> that application can have its own way of log collection.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
