You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by abhishekagarwal87 <gi...@git.apache.org> on 2016/05/02 14:20:26 UTC
[GitHub] storm pull request: STORM-1755: Revert the kafka client version to...
GitHub user abhishekagarwal87 opened a pull request:
https://github.com/apache/storm/pull/1386
STORM-1755: Revert the kafka client version to 0.8.x in storm-kafka
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/abhishekagarwal87/storm STORM-1755
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1386.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 #1386
----
commit 267ea8e38fdad7c405ecdc363917f3f8d88120c0
Author: Abhishek Agarwal <ab...@inmobi.com>
Date: 2016-05-02T12:18:06Z
STORM-1755: Revert the kafka client version to 0.8.x in storm-kafka module
----
---
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: STORM-1755: Revert the kafka client version to...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-219405686
Shouldn't this also go on 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 #1386: STORM-1755: Revert the kafka client version to 0.8.x in s...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the issue:
https://github.com/apache/storm/pull/1386
@harshach - Raised #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 pull request: STORM-1755: Revert the kafka client version to...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216590463
@abhishekagarwal87 what exactly are we trying to do here. Are we talking about the producer api not being comptabile with 0.8.x? . There was a suggested approach in storm-kafka-client patch where we decided to move producer-api under storm-kafka-client and make that version as 0.9.
If users are intended to run a storm-kafka bolt with 0.8.x cluster they should be using previous version and we should note this release notes. Just making the verison change here is a temporary fix and also we are removing any potential new features in kafka client api out of user hands , example security.
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-219406288
This should. I think it can be merged into master, 1.x and 1.0.x without any conflicts. Whoever is merging it, please let me know if I have to raise a PR for master branch.
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-219200077
Hoping this is concluded and can be merged now.
---
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: STORM-1755: Revert the kafka client version to 0.8.x in ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/1386
Please note that users cannot use previous version of storm-kafka (say, 0.10.x) since we changed package name of storm-core and so. Having profiles would be better but default should be 0.8.
---
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 #1386: STORM-1755: Revert the kafka client version to 0.8.x in s...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1386
@abhishekagarwal87 are you planning a sending a follow-up patch for this.
---
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 #1386: STORM-1755: Revert the kafka client version to 0.8.x in s...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1386
@abhishekagarwal87 didn't see this merged in. Instead of overriding the kafka.version for storm-kafka lets add another param to pom.xml and keep it at 0.8.2.1 and use that as kafka version for storm-kafka. This way distributors can override the kafka version for both storm-kafka and storm-kafka-client by passing kafka.version and new param for storm-kafka instead of keep building it 0.8.2.1 and for apache it will work as this patch intended.
---
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: STORM-1755: Revert the kafka client version to...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-217418115
Going by the Kafka coding guidelines (http://kafka.apache.org/coding-guide.html), it looks like client libraries can only be assumed to support two major (0.x) releases (current and previous). I'm wondering if it would be possible to spin off the components executing requests against Kafka into separate projects, so users can declare dependency on the newest storm-kafka-client while also retaining support for older brokers?
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216762280
It's about both the consumer and producer api. 0.9.x client (producer + consumer) is not backward compatible with 0.8.x broker.
Reverting this version change has two purposes-
1. Upgrading to new version of storm-kafka will work with 0.8.x cluster without any surprises like
https://mail-archives.apache.org/mod_mbox/storm-user/201605.mbox/%3CCAFwSPC1072CkOWsGB48HBOoMx%3D5h88AJ22sCBcJ31cB1Uqg52w%40mail.gmail.com%3E.
2. More importantly, by explicitly setting version to 0.8.x, we will ensure that in future no changes are made to storm-kafka which are not compatible with older kafka client (since compilation itself will fail). In fact, I will also add this comment in the storm-kafka pom so that kafka version is not upgraded in future.
By the way, version is being reverted only for storm-kafka module. The new kafka client for storm (storm-kafka-client) still uses 0.9.x. I think we should encourage the use of this module if user need features specific to 0.9.x version. We already have the consumer api available there. we can do the same for producer api. Happy to take it up.
---
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 #1386: STORM-1755: Revert the kafka client version to 0.8.x in s...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the issue:
https://github.com/apache/storm/pull/1386
@harshach do you mean to suggest something like following
```
<properties>
<!-- used by old kafka client -->
<storm.kafka.version>0.8.2.1</storm.kafka.version>
<storm.kafka.artifact.id>kafka_2.10</storm.kafka.artifact.id>
<!-- used by new kafka client -->
<storm.kafka.client.version>0.9.0.1</storm.kafka.client.version>
<storm.kafka.client.artifact.id>kafka_2.11</storm.kafka.client.artifact.id>
</properties>
```
I am fine with this approach. what is the naming convention to be followed? (storm.kafka and storm.kafka.client is a bit confusing)
---
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: STORM-1755: Revert the kafka client version to...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-217683094
@abhishekagarwal87 got it. But its problematic for someone who wants to build against the current version of kafka. Agree in this case that building with old version and using that would work for newer version of kafka as well and given that old api is marked as deprecated from Kafka 0.10 onwards . This approach is ok . But Ideally we should use maven profiles to build against different versions of kafka api.
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216932592
@harshach - simple consumer api may not have changed but there is a change in protocol. Using storm-kafka 1.0.0 with 0.9.x kafka cluster causes the illegalArgumentException mentioned in the storm mailing list link I pasted.
---
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 #1386: STORM-1755: Revert the kafka client version to 0.8.x in s...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1386
@abhishekagarwal87 yes. I don't have any better name in mind :) . how about keeping kafka.version as it is for new kafka spout and adding storm-kafka.kafka.version for the old one.
---
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: STORM-1755: Revert the kafka client version to...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216460875
+1
This and ES case indicate that we would be better to have some strategies or policies to be compatible with non-compatible versions range of external project.
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216558668
Yes. Good thing is storm-kafka doesn't use any api only available in kafka 0.9.x and thus can still run with kafka 0.8.x by simply excluding dependency.
---
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: STORM-1755: Revert the kafka client version to 0.8.x in ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1386
---
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: STORM-1755: Revert the kafka client version to...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-217697370
@harshach - If in future, we want to make use any incompatible kafka api in storm-kafka, that's exactly what we should do. But the chances are less likely, since we already have storm-kafka-client based off new kafka apis.
---
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: STORM-1755: Revert the kafka client version to...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/1386#issuecomment-216930761
@abhishekagarwal87 how is consumer api is relevant here. We are using simpleConsumer which is old api and it didn't changed from 0.8.x release onwards.
---
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.
---