-
Notifications
You must be signed in to change notification settings - Fork 267
Conversation
…Intermediate/Final flat map distinction
…into feature/storm_new_planner Conflicts: summingbird-core/src/test/scala/com/twitter/summingbird/TestGraphGenerators.scala
store.asInstanceOf[StoreFactory[K, W]]) | ||
|
||
def foldOperations(producers: List[Producer[Storm, _]]): FlatMapOperation[Any, Any] = { | ||
producers.foldLeft(FlatMapOperation.identity[Any]) { |
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 need to make sure andThen
on identity is free. We need to override andThen on the identity to just return the right hand argument.
// We also drop all StormNodes with no members(may occur when we visit a node already seen and its the first in that Node) | ||
val revsersedNodeSet = stormNodeSet.filter(_.members.size > 0).foldLeft(List[StormNode]()){(nodes, n) => n.reverse :: nodes} | ||
// We also drop all Nodes with no members(may occur when we visit a node already seen and its the first in that Node) | ||
val revsersedNodeSet = stormNodeSet.filter(_.members.size > 0).foldLeft(List[Node]()){(nodes, n) => n.reverse :: nodes} |
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.
is that reveresedNodeSet?
@johnynek since this is all refactored around a in the other open PR i put the spellcheck name into the other one |
def dependantsOf(n: StormNode): List[StormNode] = dependsOnM.get(n).getOrElse(List()) | ||
def dependsOn(n: StormNode): List[StormNode] = dependantOfM.get(n).getOrElse(List()) | ||
def dependantsOf(n: Node): List[Node] = dependsOnM.get(n).getOrElse(List()) | ||
def dependsOn(n: Node): List[Node] = dependantOfM.get(n).getOrElse(List()) |
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.
rename dependsOn to dependenciesOf?
Also, the right hand side looks reversed. Is it right? Can you explain this in a comment?
// TODO (https://github.com/twitter/summingbird/issues/86): | ||
// support topologies that don't end with a sum | ||
populate(topologyBuilder, summer, name) | ||
stormDag.nodes.map { node => |
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.
.foreach
Storm platform now uses the newly generated graphs