[GitHub] storm pull request #2322: Introduce config params to use latest EH client, c...
GitHub user raviperi opened a pull request: https://github.com/apache/storm/pull/2322 Introduce config params to use latest EH client, control request prefetch size, batch size of events received per call. -Refactor the code to group classes more appropriately -Remove redundant types -Javadoc comments where applicable -Preftch config parameter to dictate EH prefetch count -config parameter to introduce sleep between spout's nexttuple calls -config parameter to retrieve a batched number of events per call to EH (opposed to single event) -New data scheme to group event payload and audit params into a single type, and expose the single type as the only tuple field to downstream bolts. You can merge this pull request into a Git repository by running: $ git pull https://github.com/raviperi/storm master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2322.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 #2322 commit e9fc45d3d604b65ab3e99895b4a7711bf1b7ab30 Author: Ravi Peri <ravip...@microsoft.com> Date: 2017-09-11T21:35:48Z Introduce config params to dictate EH request prefetch size, max number of events received per call. -Refactor the code to group classes more appropriately -Remove redundant types -Javadoc comments where applicable -Preftch config parameter to dictate EH prefetch count -config parameter to introduce sleep between spout's nexttuple calls -config parameter to retrieve a batched number of events per call to EH (opposed to single event) -New data scheme to group event payload and audit params into a single type, and expose the single type as the only tuple field to downstream bolts. ---
[GitHub] storm pull request #1738: STORM-2127: Storm-eventhubs should use latest amqp...
GitHub user raviperi opened a pull request: https://github.com/apache/storm/pull/1738 STORM-2127: Storm-eventhubs should use latest amqp and eventhubs-client versions STORM-2127: Storm-eventhubs should use latest amqp and eventhubs-client versions You can merge this pull request into a Git repository by running: $ git pull https://github.com/raviperi/storm 1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1738.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 #1738 commit 60e6eb9dd685f1ee434065f0de80bd4b1e45bef6 Author: Ravi Peri <ravip...@microsoft.com> Date: 2016-10-17T22:59:56Z STORM-2127: Storm-eventhubs should use latest amqp and eventhubs-client versions --- 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. ---
[GitHub] storm issue #1702: STORM-2127: Storm-eventhubs should use latest amqp and ev...
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 @harshach Updated title as per your recommendation. --- 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. ---
[GitHub] storm issue #1717: STORM-2127: Storm-eventhubs should use latest amqp and ev...
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1717 @harshach Updated title as per your recommendation. --- 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. ---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 @revans2 Please let me know if you have further comments. --- 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. ---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r81616391
--- Diff:
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/StringEventDataScheme.java
---
@@ -0,0 +1,69 @@
+/***
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+
***/
+package org.apache.storm.eventhubs.spout;
+
+import org.apache.storm.tuple.Fields;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.qpid.amqp_1_0.client.Message;
+import org.apache.qpid.amqp_1_0.type.Section;
+import org.apache.qpid.amqp_1_0.type.messaging.AmqpValue;
+import org.apache.qpid.amqp_1_0.type.messaging.ApplicationProperties;
+import org.apache.qpid.amqp_1_0.type.messaging.Data;
+import org.apache.storm.tuple.Fields;
+
+/**
+ * An Event Data Scheme which deserializes message payload into the
Strings.
+ * No encoding is assumed. The receiver will need to handle parsing of the
+ * string data in appropriate encoding.
+ *
+ * Note: Unlike other schemes provided, this scheme does not include any
+ * metadata.
+ *
+ * For metadata please refer to {@link BinaryEventDataScheme}, {@link
EventDataScheme}
+ */
+public class StringEventDataScheme implements IEventDataScheme {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public List deserialize(Message message) {
+final List fieldContents = new ArrayList();
+
+for (Section section : message.getPayload()) {
+ if (section instanceof Data) {
+Data data = (Data) section;
+fieldContents.add(new String(data.getValue().getArray()));
--- End diff --
Yes, It is an important change, and I have that change in the pipeline.
I will send a different PR for that change, as I want to make sure that it
does not break any assumptions client code and our own SCP.Net framework make.
---
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.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r81605840 --- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java --- @@ -121,8 +121,8 @@ public void preparePartitions(Map config, int totalTasks, int taskIndex, SpoutOu zkEndpointAddress = sb.toString(); } stateStore = new ZookeeperStateStore(zkEndpointAddress, - (Integer)config.get(Config.STORM_ZOOKEEPER_RETRY_TIMES), - (Integer)config.get(Config.STORM_ZOOKEEPER_RETRY_INTERVAL)); + Integer.parseInt(config.get(Config.STORM_ZOOKEEPER_RETRY_TIMES).toString()), --- End diff -- It compiles :) The issue was that the values were being parsed as Long, and not Integer. Instead of doing an explicit cast, I wanted to be safe and parse the numeric value. --- 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. ---
[GitHub] storm issue #1717: Update eventhub client dependency. Move Storm-Eventhubs d...
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1717 Below JIRA that reflects the need for the above work. STORM-2127 - Storm-eventhubs should use latest amqp and eventhubs-client versions --- 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. ---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 Created a new PR https://github.com/apache/storm/pull/1717 for the master branch. Created a new JIRA that reflects the need for the aboce work. STORM-2127 - Storm-eventhubs should use latest amqp and eventhubs-client versions --- 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. ---
[GitHub] storm pull request #1717: Update eventhub client dependency. Move Storm-Even...
GitHub user raviperi opened a pull request: https://github.com/apache/storm/pull/1717 Update eventhub client dependency. Move Storm-Eventhubs dependency ve⦠Update eventhub client dependency. Move Storm-Eventhubs dependency vesion definitions to parent pom. Introduce new schemes for message handling You can merge this pull request into a Git repository by running: $ git pull https://github.com/raviperi/storm master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1717.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 #1717 commit 362858c49919d2264606be06469f1da79fadc511 Author: Ravi Peri <ravip...@microsoft.com> Date: 2016-09-27T21:06:16Z Update eventhub client dependency. Move Storm-Eventhubs dependency version definitions to parent pom. Introduce new schemes for message handling --- 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. ---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on the issue: https://github.com/apache/storm/pull/1702 The latest PR addresses the above comments . Move dependencies to parent pom . Revert changes to versions to reflect naming convention. I will be sending a different PR to master branch, that includes the above changes. --- 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. ---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80753947 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 ../../pom.xml storm-eventhubs -1.0.3-SNAPSHOT +1.0.2.1 --- End diff -- Got it. Reverted it to match the version convention. --- 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. ---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user raviperi commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80753320 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 --- End diff -- Reverted them. I had initially changed it to validate compatibility with 1.0.2 binaries. --- 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. ---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
GitHub user raviperi opened a pull request: https://github.com/apache/storm/pull/1702 Update Eventhub-Client jar dependency to 1.0.1 Eventhub client library has few key fixes that need to be included in storm-eventhub jar. Fixes: 1. Fix for EventHubSender may throw NullPointerException on .close() 2. EventHubsSender keeps failing to send messages once it hits ConnectionClosedException You can merge this pull request into a Git repository by running: $ git pull https://github.com/raviperi/storm 1.0.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1702.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 #1702 commit ec296d2a6e79f17917a65364b46e3c5d4ea3f09e Author: Ravi Peri <ravip...@microsoft.com> Date: 2016-09-22T00:21:06Z Update Eventhub-Client jar dependency to 1.0.1 --- 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. ---
