You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2019/01/30 07:22:19 UTC
[GitHub] igalshilman commented on a change in pull request #7590:
[FLINK-11329][core] Migrating CompositeSerializers
igalshilman commented on a change in pull request #7590: [FLINK-11329][core] Migrating CompositeSerializers
URL: https://github.com/apache/flink/pull/7590#discussion_r252139840
##########
File path: flink-end-to-end-tests/flink-stream-state-ttl-test/src/main/java/org/apache/flink/streaming/tests/verify/ValueWithTs.java
##########
@@ -56,8 +57,8 @@ public String toString() {
/** Serializer for Serializer. */
public static class Serializer extends CompositeSerializer<ValueWithTs<?>> {
- public Serializer(TypeSerializer<?> userValueSerializer) {
- super(true, userValueSerializer, LongSerializer.INSTANCE);
+ public Serializer(TypeSerializer<?> valueSerializer, TypeSerializer<Long> timestampSerializer) {
Review comment:
As far as I can tell this is not a user facing serializer, rather used internally by the `TtlSerializer`,
and I think it is important to make it explicit that this is a composite serializer and these are the nested serializers that define it.
As a way to reduce verbosity, we can add a static factory method with an explicit name.
Feel free to decide for yourself :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services