Github user haewanj commented on a diff in the pull request:
https://github.com/apache/storm/pull/2177#discussion_r123888471
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/NamedSubscription.java
---
@@ -56,6 +57,6 @@ public NamedSubscription(String ... topics) {
@Override
public String getTopicsString() {
- return String.valueOf(topics);
+ return StringUtils.join(topics, ",");
--- End diff --
Thanks for the suggestion.
As you know, To keep java 1.7+ build, 'String.join()' is not a solution
like master branch. And 'storm-core' has dependency both commons-lang and
guava. So I choose commons-lang but If to pass the test that travis
MODUELS='!storm-core', storm-kafka-client needs the dependency even Guava too.
That's why I added commons-lang to storm-kafka-client's pom file.
If avoid adding a new dependency in storm-kafka-client, we can code maybe
like this
StringBuilder sb = new StringBuilder();
for(String topic: topics) {
sb.append(topic).append(",");
}
sb.deleteCharAt(sb.lastIndexOf(","));
return sb.toString();
Many of external/packages have commons-lang dependency so, I thought it was
fine.
By the way It has not been passed the Integration-test on travis-ci, I have
no idea. but in my local integration-test is fine. (ex: mvn -P
integration-tests-only integration-test)
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
