[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16156540#comment-16156540 ] ASF GitHub Bot commented on FLINK-7440: --- Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/4537 > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16126800#comment-16126800 ] ASF GitHub Bot commented on FLINK-7440: --- Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/4537 @bowenli86 @zentol Thanks a lot for the reviews! I will address your comments and merge this. > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16126768#comment-16126768 ] ASF GitHub Bot commented on FLINK-7440: --- Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/4537 Once this is checked in, we'll be more confident that https://github.com/apache/flink/pull/4473 is all good and it doesn't break anything > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125951#comment-16125951 ] ASF GitHub Bot commented on FLINK-7440: --- Github user bowenli86 commented on a diff in the pull request: https://github.com/apache/flink/pull/4537#discussion_r13250 --- Diff: flink-connectors/flink-connector-kinesis/src/test/java/org/apache/flink/streaming/connectors/kinesis/FlinkKinesisConsumerTest.java --- @@ -1062,4 +1104,28 @@ public boolean isClearCalled() { return fakeRestoredState; } + + private final class NonSerializableDeserializationSchema implements KinesisDeserializationSchema { --- End diff -- Can you please add a comment explaining why this is not serializable and why the following one is serializable? Basically pointing out this class is not static. > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125575#comment-16125575 ] ASF GitHub Bot commented on FLINK-7440: --- Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/4537#discussion_r132932684 --- Diff: flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/FlinkKinesisConsumer.java --- @@ -176,6 +177,10 @@ public FlinkKinesisConsumer(List streams, KinesisDeserializationSchema Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125344#comment-16125344 ] ASF GitHub Bot commented on FLINK-7440: --- Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/4537 R: @bowenli86 would you like to take a look at the changes here? > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (FLINK-7440) Add eager serializable checks on provided de-/serialization schemas for Kinesis consumer / producer
[ https://issues.apache.org/jira/browse/FLINK-7440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125341#comment-16125341 ] ASF GitHub Bot commented on FLINK-7440: --- GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/4537 [FLINK-7440] [kinesis] Add various eager serializability checks on to Kinesis connector ## What is the purpose of the change This is a minor improvement for user experience for the Kinesis consumer and producer. If the provided `KinesisDeserializationSchema`, `KinesisSerializationSchema`, `KinesisPartitioner` is not serializable, a more clear error message is shown instead of some vague "the implementation of RichSunkFunction is not serializable". ## Brief change log Changes are broken up into 2 commits: - for consumer: eagerly check serializability of `KinesisDeserializationSchema`, with better error msgs - for producer: eagerly check serializability of `KinesisSerializationSchema` and `KinesisPartitioner`, with better error msgs. ## Verifying this change Added new tests to both `FlinkKinesisConsumerTest` and `FlinkKinesisProducer` test, to verify that the expected error message is thrown (and also not thrown if the provided arg instances are serializable). Also included tests to check that `FlinkKinesisConsumer` and `FlinkKinesisProducer` themselves are serializable. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): **no** - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: **no** - The serializers: **no** - The runtime per-record code paths (performance sensitive): **no** - Anything that affects deployment or recovery: **no** ## Documentation - Does this pull request introduce a new feature? **no** - If yes, how is the feature documented? **not applicable** You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-7440 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4537.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4537 commit 96f0799aaece6ae226aeef17ff96c2fe2debc3e0 Author: Tzu-Li (Gordon) Tai Date: 2017-08-14T07:16:54Z [FLINK-7440] [kinesis] Eagerly check serializability of deserialization schema in FlinkKinesisConsumer This commit also adds tests for verifying that the FlinkKinesisConsumer itself is serializable. commit 9faee3dc530c9b9021e33121f502f1e67deceaf0 Author: Tzu-Li (Gordon) Tai Date: 2017-08-14T07:35:43Z [FLINK-7440] [kinesis] Eagerly check that provided schema and partitioner is serializable in FlinkKinesisProducer This commit also adds a test to verify that the FlinkKinesisProducer is serializable. > Add eager serializable checks on provided de-/serialization schemas for > Kinesis consumer / producer > --- > > Key: FLINK-7440 > URL: https://issues.apache.org/jira/browse/FLINK-7440 > Project: Flink > Issue Type: Improvement > Components: Kinesis Connector >Reporter: Tzu-Li (Gordon) Tai >Assignee: Tzu-Li (Gordon) Tai > Fix For: 1.4.0, 1.3.3 > > > For better user experience, we should add eager serializable checks on the > provided {{KinesisDeserializationSchema}} / {{KinesisSerializationSchema}}, > with better error messages pointing out exactly that the serialization schema > isn't serializable. -- This message was sent by Atlassian JIRA (v6.4.14#64029)