You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by hmcl <gi...@git.apache.org> on 2016/07/12 15:18:17 UTC
[GitHub] storm pull request #1556: STORM-1737: storm-kafka-client has compilation err...
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1556
STORM-1737: storm-kafka-client has compilation errors with Apache Kafka 0.10
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hmcl/storm-apache STORM-1737
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1556.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 #1556
----
commit 04f00e56beb9799dc8b8fc732271b786ced5c21e
Author: Hugo Louro <hm...@gmail.com>
Date: 2016-04-29T16:18:05Z
STORM-1737: storm-kafka-client has compilation errors with Apache Kafka 0.10
----
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
+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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR Yes. Users can still use the storm-kafka connector (old api) to connect to any version of kafka. Given that this is new consumer api we better move to compatible API soon. Hence the reason to push this as part of 1.0.2 release.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
What's our plan to support version lines of Kafka? I think we need to guide storm-kafka to users that for 0.8 you need to blabla, and for 0.9, and for 0.10.
And storm-kafka-client is no longer compatible with 0.9, it would be better to go with [STORM-1876](https://issues.apache.org/jira/browse/STORM-1876) (#1482). If this patch merged in without STORM-1876, 0.9 users should download the code, and **change directory to external/storm-kafka**, and build module with proper `kafka.version` and `kafka.artifact.id` parameters.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@hmcl can you update the README with a Note: supports from kaka-client 0.10 version on wards
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR we released 1.0 like few weeks back and even they are using the new consumer api in 0.10 will be api compatible with 0.9 as well it just the parameter types that are 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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
+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 pull request #1556: STORM-1737: storm-kafka-client has compilation err...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1556#discussion_r70542939
--- Diff: external/storm-kafka/src/test/org/apache/storm/kafka/KafkaTestBroker.java ---
@@ -17,21 +17,21 @@
*/
package org.apache.storm.kafka;
+import org.apache.commons.io.FileUtils;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.imps.CuratorFrameworkState;
import org.apache.curator.retry.ExponentialBackoffRetry;
import org.apache.curator.test.InstanceSpec;
import org.apache.curator.test.TestingServer;
-import kafka.server.KafkaConfig;
-import kafka.server.KafkaServerStartable;
-import org.apache.commons.io.FileUtils;
-
import java.io.File;
import java.io.IOException;
import java.util.Properties;
+import kafka.server.KafkaConfig;
+import kafka.server.KafkaServerStartable;
+
/**
--- End diff --
Out of scope, but we need to get rid of this comment.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
And let's clarify: what I'm thinking about is not only API level compatibility but Kafka client - broker version compatibility. We modified storm-kafka version to 0.8 due to this issue.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR 0.9 users won't be able to use the new API anyway. So why are we worried about that part. ). 0.9 users can still use storm-kafka but the new kafka consumer in storm-kafka-client
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@abhishekagarwal87 We can put that in the README on which version we are going to support for storm-kafka-client.
agree on making the dependencies provided.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by hmcl <gi...@git.apache.org>.
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1556
@harshach @HeartSaVioR @abhishekagarwal87, thank you for your feedback. The updated ReadME, including the added Wildcard Support will be out in a few mins.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the issue:
https://github.com/apache/storm/pull/1556
I will make the scope changes in #1482
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
Where we would want to update support versions on storm-kafka-client?
I'd be +1 when we're clear to where we handle that thing.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR I don't think this patch need to wait on storm-kafka changes in STORM-1876. Irrespective both these patches storm-kafka will work against 0.8, 0.9 and 0.10
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
@hmcl Please update README.md to describe this module is compatible with 0.10.0.
@harshach We're having potential users which already uses storm-kafka-client with Kafka 0.9. It might be better for them to guide not updating their storm-kafka-client version as same as storm-core due to client version change, or dropping usage of storm-kafka-client and back to storm-kafka.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR As stated before we can add this to README of storm-kafka-client
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
@harshach
Sorry I've confused with compatibility between client and broker version. Seems like kafka 0.9 broker is compatible with kafka 0.8 client. I guess you're based on this. Is it correct?
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1556
Sorry to not clear on `where`: I mean which PR (or new issue) will handle updating the doc. Now I'm +1 once @hmcl updates the doc.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the issue:
https://github.com/apache/storm/pull/1556
@harshach just wanted to make sure that compatibility of kafka 0.10 with 0.9 broker has been tested.
Also given that there are three version lines of kafka client and two different kafka modules within storm, we should clearly document which combinations of kafka-client are supported.
e.g.
storm-kafka supports kafka client 0.8.x and 0.9.x
storm-kafka-client supports kafka client 0.10.x
Apart from that kafka dependencies should be marked provided, so that users are forced to chose a kafka client version explicitly. Whether the kafka client version chosen by user works with broker, should be left for user to decide.
---
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 #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1556
@HeartSaVioR is this ok to be merged in.
---
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 #1556: STORM-1737: storm-kafka-client has compilation err...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1556
---
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.
---