[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/5567 @StephanEwen a tuple arity would be enough if the tuple serializer would only serialize tuples but it also serializes case classes and POJOs. Wouldn't be the fully qualified name better? But yes writing classes should be avoided. ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5567 +1 to what Gordon suggested. Let's move towards avoiding writing classes alltogether in any config snapshot. Tuple arity is the right thing for tuples, plus ideally a format version number. ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/5567 @tzulitai exactly, I think tuples/case classes are just a special case. @aljoscha are we going to merge this now? (given the checkstyle error is fixed) ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/5567 @twalthr I see. Then for other Scala types whose serializer config snapshot does not require writing classes, we probably don't need to include them in the `FailureTolerantObjectInputStream` then. ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/5567 @tzulitai I think just writing out the arity is not enough. It really depends on the type of tuple. Maybe it would have been better to write out only the name of the class. But since we support tuple POJOs this might be a little bit risky. The tuple serializer serializes general subclasses for both Java and Scala and case classes. I tested case classes, tuples, `Either` and `Unit`. What do you think we should test as well? The other serializers do not include classes that need to be snapshotted. ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/5567 A few side notes: 1) Maybe we should have written just the arity of tuples in the config snapshots. That would have avoided this issue. 2) Is there any potential cases like this one, for other Scala types beside Tuples? ---
[GitHub] flink issue #5567: [FLINK-8451] [serializers] Make Scala tuple serializer de...
Github user twalthr commented on the issue: https://github.com/apache/flink/pull/5567 CC @tzulitai @aljoscha ---
