You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by raviperi <gi...@git.apache.org> on 2016/09/22 00:28:53 UTC
[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 <ra...@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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1702: Jira 2127- Storm-eventhubs should use latest amqp and eve...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1702
@raviperi sorry for nitpick. we need the format of the title to be
" 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80771003
--- 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<Object> deserialize(Message message) {
+ final List<Object> fieldContents = new ArrayList<Object>();
+
+ for (Section section : message.getPayload()) {
+ if (section instanceof Data) {
+ Data data = (Data) section;
+ fieldContents.add(new String(data.getValue().getArray()));
--- End diff --
I don't remember the details of these message formats, but you should probably specify an encoding for this 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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80776662
--- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java ---
@@ -152,7 +152,7 @@ public void open(Map config, TopologyContext context, SpoutOutputCollector colle
try {
preparePartitions(config, totalTasks, taskIndex, collector);
} catch (Exception e) {
- logger.error(e.getMessage());
+ collector.reportError(e);
--- End diff --
Thanks @revans2
---
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] storm issue #1702: STORM-2127: Storm-eventhubs should use latest amqp and ev...
Posted by raviperi <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r81609543
--- 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 --
Sorry, misread this bit. Read it as parseInt(conf).toString instead of parseInt(conf.toString) :)
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80771322
--- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java ---
@@ -152,7 +152,7 @@ public void open(Map config, TopologyContext context, SpoutOutputCollector colle
try {
preparePartitions(config, totalTasks, taskIndex, collector);
} catch (Exception e) {
- logger.error(e.getMessage());
+ collector.reportError(e);
--- End diff --
Someone else can correct me here, but I don't think collector.reportError puts anything in the log, so it might be nice to have both this and logger.error
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r79990064
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -21,19 +21,19 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
--- End diff --
These versions probably shouldn't be changed
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1702
Hm, nevermind the new client, it requires Java 8. It's still an option for Storm 2.0 though.
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80133950
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -21,19 +21,19 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
--- End diff --
Yes @srdo is right.
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1702
I am not really familiar with EventHub all that much so I don't really feel all that confident in reviewing it. I mostly jumped on to answer a few generic questions.
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80133790
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -21,19 +21,19 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<artifactId>storm-eventhubs</artifactId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2.1</version>
<packaging>jar</packaging>
<name>storm-eventhubs</name>
<description>EventHubs Storm Spout</description>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <eventhubs.client.version>0.9.1</eventhubs.client.version>
+ <eventhubs.client.version>1.0.1</eventhubs.client.version>
--- End diff --
IMHO it might be better to maintain the version of dependencies from the root pom.xml.
Some modules do, and some modules don't, so it's not a mandatory but just food for thought.
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1702
No, this seems fine. +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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1702
@raviperi do you have JIRA filed here for this https://issues.apache.org/jira/browse/STORM/
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1702
Btw, IMO, upgrading the dependency is worth to file an issue.
@raviperi Could you file it and copy description of PR to issue's description?
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80720194
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -21,19 +21,19 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<artifactId>storm-eventhubs</artifactId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2.1</version>
<packaging>jar</packaging>
<name>storm-eventhubs</name>
<description>EventHubs Storm Spout</description>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <eventhubs.client.version>0.9.1</eventhubs.client.version>
+ <eventhubs.client.version>1.0.1</eventhubs.client.version>
--- End diff --
I agree having them in the root gives everyone a consistent view of dependencies. It would be nice to move the version there.
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80770524
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -1,113 +1,120 @@
<?xml version="1.0" encoding="UTF-8"?>
-<!--
- 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
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
--- End diff --
Nitpick: Your editor seems to have replaced spaces with tabs, which has made the diff much larger than it needs to be. Could you reformat these files back to something closer to the original formatting?
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<artifactId>storm-eventhubs</artifactId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2.1</version>
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1702
Is there a similar pull request to master?
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80133198
--- Diff: external/storm-eventhubs/pom.xml ---
@@ -21,19 +21,19 @@
<parent>
<artifactId>storm</artifactId>
<groupId>org.apache.storm</groupId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2</version>
<relativePath>../../pom.xml</relativePath>
</parent>
<artifactId>storm-eventhubs</artifactId>
- <version>1.0.3-SNAPSHOT</version>
+ <version>1.0.2.1</version>
--- End diff --
Here too. Every modules should follow the Storm version.
Would be better to remove this line as same as other modules.
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1702
This seems fine once the storm versions are reverted, though you should get this on the master branch first. I'd like to mention before you spend too much time on trying to make the spout run reliably, that we've had some reliability problems with the com.microsoft.eventhubs.client:eventhubs-client since the underlying AMQP library isn't being maintained. There's a new eventhub client here based on Apache Proton which is likely to provide a better experience. https://github.com/Azure/azure-event-hubs/tree/master/java
---
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] storm issue #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1702
+1. @srdo do you have any further comments pending.
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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<Object> deserialize(Message message) {
+ final List<Object> fieldContents = new ArrayList<Object>();
+
+ 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1702#discussion_r80775363
--- Diff: external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubSpout.java ---
@@ -152,7 +152,7 @@ public void open(Map config, TopologyContext context, SpoutOutputCollector colle
try {
preparePartitions(config, totalTasks, taskIndex, collector);
} catch (Exception e) {
- logger.error(e.getMessage());
+ collector.reportError(e);
--- End diff --
It does log it, and puts it on the UI.
https://github.com/apache/storm/blob/28563ece16274bcd61827f38e960ce2d27a57dea/storm-core/src/jvm/org/apache/storm/executor/error/ReportError.java#L61
There is throttling around it so if we get too many errors too quickly they will not all be logged and not all of them will be written to ZK.
---
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] storm pull request #1702: STORM-2127: Storm-eventhubs should use latest amqp...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1702
---
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] storm pull request #1702: Update Eventhub-Client jar dependency to 1.0.1
Posted by raviperi <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---