search

twitter / scalding

A Scala API for Cascading
http://twitter.com/scalding
Apache License 2.0
3.5k stars 708 forks source link

Cherrypick for 0.17.3 #1730

Closed johnynek closed 7 years ago

johnynek commented 7 years ago

what do you think @ttim @ianoc @pankajroark ?

If you give the +1, I'll publish this as 0.17.3

I don't think anything else can go in as it is not binary compatible.

johnynek commented 7 years ago

cc @tdyas

tdyas commented 7 years ago

thanks!

ianoc commented 7 years ago

👍

johnynek commented 7 years ago

@tdyas 0.17.3 should be making its way to maven central now. Let us know if you learn any more about these null counters. I really don't like silently dropping the counters. Maybe in the future we will add a configuration flag like "optional counters" which allows it to silently ignore counters and otherwise fails if we get a null reporter.

tdyas commented 7 years ago

Thanks. This will let me get off our local fork of Scalding.

As for tracking down the cause, we did track it to a specific changeset although the "why" still eludes me. The "weird" part of the changeset in question was that the job had a Option[TypedPipe] as a val at the toplevel of the job which was later converted to either a transformed version of the wrapped TypedPipe getOrElse TypedPipe.empty. (The developer was trying to optionally include prior data into their computation.) That could be a total red herring though, I haven't gotten far enough into the issue to know any more.

I agree regarding a config flag. Obviously, I need to work around this issue, but from a correctness standpoint, the need to do that raises a red flag.