JsonLine should handle empty lines #966
Conversation
Currently it fails while parsing
| val values = splitFields.map { nestedRetrieval(Option(fs), _) } | ||
| new cascading.tuple.Tuple(values: _*) | ||
| pipe.collectTo[String, Tuple]('line -> fields) { | ||
| case line: String if line.trim.nonEmpty => |
There was a problem hiding this comment.
I'm wondering if this should be configurable? Are there cases where you might simply want to fail in this case?
There was a problem hiding this comment.
Good question, but by default it can be assumed that generating lines of JSON will at least generate some trailing lines which should not make it fail. If you wish I can add a constructor parameter.
There was a problem hiding this comment.
So, should I modify it, or can it be merged the way it is?
There was a problem hiding this comment.
Can you modify it and have the current behavior remain constant with what it is? It avoids this being a breaking change on users at run time.
…ing previous behavior
…velop Conflicts: scalding-json/src/main/scala/com/twitter/scalding/JsonLine.scala scalding-json/src/test/scala/com/twitter/scalding/JsonLineTest.scala
|
Well rebase didn't help avoid the merge conflicts, guess I'll have to redo it. |
|
Thanks for taking the time here to address our concerns. |
JsonLine should handle empty lines
Currently, JsonLine fails on empty / white lines. I think it'd be quite convenient to simply ignore them.