You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by bhupeshchawda <gi...@git.apache.org> on 2016/05/16 17:54:19 UTC
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
GitHub user bhupeshchawda opened a pull request:
https://github.com/apache/incubator-apex-malhar/pull/275
APEXMALHAR-2084-Fixed getters and setters for topics and clusters
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bhupeshchawda/incubator-apex-malhar APEXMALHAR-2084-kafka-setter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-malhar/pull/275.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #275
----
commit 01db1dd31a3e895d8db1800c7b1a20e9e1221012
Author: bhupesh <bh...@gmail.com>
Date: 2016-05-13T23:48:53Z
APEXMALHAR-2084-Fixed getters and setters for topics and clusters
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by bhupeshchawda <gi...@git.apache.org>.
Github user bhupeshchawda commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63427002
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
The clusters are separated by commas. In case the clusters themselves have multiple host:port pairs, the string can be specified using quotes as follows: "host1:port1, host2:port2", "host3:port3".
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63432024
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
But does it work in properties.xml file? Can you give an example?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63558782
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
Have you tested it? If so, would it make sense to include it into the javadoc?
@siyuanh does this address the issue you were trying to fix previously?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63406074
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
}
- public void setTopics(String topics)
+ public String[] getClusters()
{
- this.topics = Iterables.toArray(Splitter.on(',').trimResults().omitEmptyStrings().split(topics), String.class);
+ return clusters;
}
/**
- * The topics the operator consumes, separate by','
+ * The topics the operator consumes
* Topic name can only contain ASCII alphanumerics, '.', '_' and '-'
*/
- public String getTopics()
+ public void setTopics(String[] topics)
+ {
--- End diff --
You can avoid creation of the intermediate ArrayList and compact the array in place with something like this but
feel free to ignore if you like the original better:
{code}
int j = 0, len = topics.length;
for (int i = 0; i < len; ++i) {
if (null == topics[i] || 0 == topics[i].length()) continue;
if (i != j) topics[j] = topics[i];
++j;
}
this.topics = Arrays.copyOfRange(topics, 0, j);
{code}
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: [Review Only] APEXMALHAR-2084-...
Posted by bhupeshchawda <gi...@git.apache.org>.
Github user bhupeshchawda closed the pull request at:
https://github.com/apache/incubator-apex-malhar/pull/275
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by bhupeshchawda <gi...@git.apache.org>.
Github user bhupeshchawda commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63432478
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
Yes, it should work in xml. This is an example configuration for clusters: `"localhost:9092, localhost:9093", "localhost2:9092"`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63561257
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
@tweise This issue is reported by @bhupeshchawda that the operator won't show up in gateway because it violates the BEAN convention. I'm ok with this change, as long as it is configurable through configuration file and probably test the json application also to make sure the operator can be used everywhere
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63426625
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
How does user separate nodes in each cluster string?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2084-Fixed getters ...
Posted by bhupeshchawda <gi...@git.apache.org>.
Github user bhupeshchawda commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/275#discussion_r63636939
--- Diff: kafka/src/main/java/org/apache/apex/malhar/kafka/AbstractKafkaInputOperator.java ---
@@ -413,33 +413,39 @@ public int getInitialPartitionCount()
return initialPartitionCount;
}
- public void setClusters(String clusters)
- {
- this.clusters = clusters.split(";");
- }
-
/**
* Same setting as bootstrap.servers property to KafkaConsumer
* refer to http://kafka.apache.org/documentation.html#newconsumerconfigs
- * To support multi cluster, you can have multiple bootstrap.servers separated by ";"
+ * To support multi cluster, you can have multiple elements in the array
*/
- public String getClusters()
+ public void setClusters(String[] clusters)
{
- return Joiner.on(';').join(clusters);
+ this.clusters = clusters;
--- End diff --
I have tested whether the configuration works in xml. This is verified using a application test.
For UI component, I have just verified that it allows for one text box per element in the array. However, I think we should wait on this pull request until I can test it out end to end for the UI.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---