Feature/summingbird config #339
Conversation
…lean it up nicely
|
@sritchie this should pass now, PR ready for review |
|
this is now based on having the general graph merged first |
…/SummingbirdConfig
|
@sritchie can you take a look at this? thanks |
| @@ -45,7 +46,7 @@ import ConfigBijection._ | |||
| // the intermediate key-values for using a store as a service | |||
| // in another job. | |||
| // Prefer using .write in the -core API. | |||
| case class StoreIntermediateData[K, V](sink: ScaldingSink[(K,V)]) extends java.io.Serializable | |||
| case class StoreIntermediateData(sink: ScaldingSink[(Any, Any)]) extends java.io.Serializable | |||
There was a problem hiding this comment.
So, I know it gets the graph walking to compile, but this makes me nervous... Doesn't this stop the StoreIntermediateData call into the builder API from type checking? Do we have an example job that still works with the Any everywhere, or proof that you can't pass in the wrong sink?
There was a problem hiding this comment.
This could only ever enforce on one line, specifying the type params on the constructor, and then the sink not matching those. -- though that appears to be how its used internally. Will commit change that brings them back. (Also hadn't realized this was in the external API...) Would have just broken users code
|
Few comments, then looks good! |
Add a commment to close 334
Handles the mutable state in the backing stores via common proxy model.
Simplify the Scalding/Storm env's so that producer API variants don't need lots of duplication