-
Notifications
You must be signed in to change notification settings - Fork 4.1k
STORM-207: Add storm-starter under examples directory #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2.2.5 is required for Twitter (Kurtis remark)
Add Maven 2 POM to build storm-starter with maven
…t.clj, update README
Align storm-starter with upstream Storm 0.9.0 (release).
This addresses ConcurrentModificationException that might be thrown in the ranker bolts, when the Rankings object would be modified (by the emitting bolt) after sending it downstream.
|
I will volunteer to be the initial sponsor/maintainer for this module. |
|
regarding examples, maybe it is worthwhile to put DemoScheduler.java and TestingApiDemo.java into storm example folder too. |
pom.xml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spacing appears to be off here.
|
I looked at the diffs from the original storm starter and it looks good to me. One minor comment about the spacing. I would like us to think a bit about how we want to package this. Do we want to include the jar in the resulting package, just not in a place that is loaded by storm by default? @xumingming I think adding in the examples you mentioned would be great, but we can probably do them on a separate pull request. |
|
@revans2 I fixed the spacing issue. Regarding packaging, I think for a binary release the source code for anything in the examples directory should be included, in addition to a the compiled jars. I'll update the pull request with a configuration for that. I also agree that the scheduler and testing examples should also be considered, albeit in a separate pull request. |
|
Yeah, adding TestApiDemo and DemoScheduler can be in a separate PR. |
|
I just updated the build for the binary distribution to include the |
|
+1 |
|
I got compilation error, related with shaded thrift? |
|
Sorry, the above is not a real issue. the latest code "storm-buildtools/maven-shade-clojure-transformer" have solved this problem. |
|
+1 |
|
It seems the comment from showed up in an unrelated issue #50 as a reference from by me since I pulled in these commits before issuing a new pull request (#51). So github is getting confused seeing storm-starter issues in commit messages and thinks they are incubator-storm issues. #50 looks to be the last of these, though. |
Simple resource limiting
BUG-44565: Add a topology for hive bolt that reads from file to test …
BUG-44565: Add a topology for hive bolt that reads from file to test …
This PR merges the storm-starter project into an
examplesdirectory (preserving commit history) and incorporates it into the maven build.