-
Notifications
You must be signed in to change notification settings - Fork 26
[PIRK-4] Add Streaming Implementation for Apache Storm #74
Conversation
…red storm test package
fixed logging and scala version issues via pom modifications; refacto…
@@ -65,6 +66,13 @@ else if (SystemConfiguration.getProperty(ResponderProps.PLATFORM).equals("spark" | |||
ComputeResponse computeResponse = new ComputeResponse(fs); | |||
computeResponse.performQuery(); | |||
} | |||
else if (SystemConfiguration.getProperty(ResponderProps.PLATFORM).equals("storm")) |
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.
we will need to abstract this out to implement this as a Strategy Pattern - the more backends we add like Flink, Spark, Beam and others in the future its only gonna get messy with multi - if-else stmts.
<dependency> | ||
<groupId>org.apache.kafka</groupId> | ||
<artifactId>kafka_2.10</artifactId> | ||
<version>0.9.0.1</version> |
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.
make this a property ${kafka.version}
Travis build fails on an Integration Test, here's the stack trace
|
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.Random; | ||
import java.util.*; |
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.
We want to avoid import *, best to specify each import explicitly.
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* <p/> |
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 ASF header should not contain markup.
True for a number of places in the PR.
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.
Not sure how that happened. I'll clean it up. (As well as the other comments you mentioned).
@@ -90,7 +127,7 @@ static boolean validateResponderProperties() | |||
} | |||
|
|||
String platform = SystemConfiguration.getProperty(PLATFORM).toLowerCase(); | |||
if (!platform.equals("mapreduce") && !platform.equals("spark") && !platform.equals("standalone")) | |||
if (!platform.equals("mapreduce") && !platform.equals("spark") && !platform.equals("storm") && !platform.equals("standalone")) |
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.
Change this to a switch stmt using ENUM - the if list is only gonna get longer with addition of other backends and streaming and batch versions for each of them.
This is an initial implementation for a streaming version of Pirk to run on Apache Storm. I am leaving it temporarily as WIP so people have a chance to look over it and add feedback. Right now there is only one integration test which runs the Storm topology 4 times with each of the different significant configuration possibilities. I wanted to unit test the bolts, but it seems not very straightforward with the way that the Pirk processing works. I'll try to add some documentation (at the minimum a diagram of the Pirk Storm topology) by early next week.