You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by srdo <gi...@git.apache.org> on 2017/08/08 12:17:48 UTC
[GitHub] storm pull request #2268: STORM-2689: Simplify dependency configuration for ...
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2268
STORM-2689: Simplify dependency configuration for storm-kafka-example…
…s, storm-kafka-client-examples and storm-elasticsearch-examples
See https://issues.apache.org/jira/browse/STORM-2689.
The basic problem is that provided.scope doesn't allow for setting a different scope for storm-client compared to the other dependencies. Right now you can either build a jar with all dependencies + storm-client, which you can only use to run LocalCluster topologies, or a jar without storm-client that is also missing a bunch of other dependencies.
We could add another Maven profile or property like we do with provided.scope for IntelliJ in order to allow users to produce a jar without e.g. kafka-clients, but I think making running the examples as simple as possible is attractive.
I think the easiest solution is to just produce a fat jar by default. This also seems to be how the rest of the examples work.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srdo/storm STORM-2689
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2268.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 #2268
----
commit 145026ec555cc543ce4603d77396eb996f2575af
Author: Stig Rohde Døssing <sr...@apache.org>
Date: 2017-08-08T11:04:04Z
STORM-2689: Simplify dependency configuration for storm-kafka-examples and storm-kafka-client-examples
----
---
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 #2268: STORM-2689: Simplify storm-kafka-example���
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
@HeartSaVioR Thanks, squashed. This should only 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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
We could get rid of the storm-starter dependency if we removed the DRPC demonstration from the Trident examples and replaced the RandomSentenceSpout. There's already a DRPC demo in storm-starter, so I don't know why we also need it here. I think it makes the example more complicated than it needs to be.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2268
+1 Nice improvement.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
For some reason the integration test can't download Zookeeper. I'm guessing it's unrelated.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
@HeartSaVioR Done. I've changed the storm-kafka(-client)-examples projects to just put some random strings in Kafka and retrieve and print them in the Trident examples.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2268
@srdo Please squash the commits into one. I'll merge after that. Thanks in advance!
---
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 #2268: STORM-2689: Simplify storm-kafka-example���
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2268
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2268
I feel that dependencies are going to be tricky in storm-kafka-examples and storm-kafka-client-examples. storm-kafka-client-examples depends on storm-kafka-examples, and storm-kafka-examples depends on storm-starter, and storm-starter depends on lots of dependencies like storm-hdfs, storm-redis, etc.
Looks like storm-kafka-client-examples refers storm-kafka-examples only for referencing `KafkaProducerTopology`. If then I think we shouldn't. There's a side effect: because of relying on KafkaProducerTopology, we don't have any code example to show how to use KafkaBolt in `storm-kafka-client`.
Let's duplicate KafkaProducerTopology with using only `storm-kafka-client` things. @hmcl WDYT?
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
ping @HeartSaVioR and @hmcl since you reviewed https://github.com/apache/storm/pull/2243 and this is related.
---
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 #2268: STORM-2689: Simplify storm-kafka-example���
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
Test failure is unrelated, flaky test in storm-jms.
---
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 #2268: STORM-2689: Simplify dependency configuration for ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2268#discussion_r131997782
--- Diff: examples/storm-kafka-examples/pom.xml ---
@@ -24,6 +24,11 @@
<version>2.0.0-SNAPSHOT</version>
<relativePath>../../pom.xml</relativePath>
</parent>
+
+ <properties>
+ <!-- Override with -Dkafka.dependency.scope=compile to generate a jar with dependencies -->
+ <kafka.dependency.scope>provided</kafka.dependency.scope>
--- End diff --
This workaround is necessary because this module is included as a dependency in storm-kafka-client-examples, so we need a normal install to generate a slim jar.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2268
@HeartSaVioR You're right, it's nicer if the example is not relying on storm-kafka. I copied the missing classes to storm-kafka-client-examples, and added a bunch of documentation.
---
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 #2268: STORM-2689: Simplify dependency configuration for storm-k...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2268
@srdo +1 to remove DRPC demo in other examples rather than storm-starter. Please go ahead. Thanks.
---
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.
---