[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann PR has been updated! I had to do some changes: - map `User` object to `String` in `TestAvroConsumerConfluent` class - use `read_messages_from_kafka` instead of `read_messages_from_kafka_avro` for vaidation as the events are `String` not `Avro` anymore - removed `kafka-avro` dependency When we add `AvroSerializationConfluentSchema` to Flink, I will update the test as we discussed. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann make sense. I created this ticket https://issues.apache.org/jira/browse/FLINK-9679 for implementing `AvroSerializationConfluentSchema`. I will make a PR for it shortly. Mean time I will update this PR also and remove `AvroSerializationConfluentSchema` and using `SimpleStringSchema` which later we can update the test. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/6083 This is a good point @medcv. However, I think we should tackle adding a `AvroSerializationConfluentSchema` as an orthogonal step. What about removing it from this PR which covers the existing integration with Confluent's schema registry. Additionally, we should open a JIRA issue to add a `AvroSerializationConfluentSchema` to Flink. Once this has been added, we can adapt this end-to-end test. What do you think @medcv? ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann here is my two cents: By converting the events to `String` and sending them to Kafka there is a high risk to produce a `bad events` and we will lose the benifit of Schema Registry to avoid this malformed events sent to the topic. what you think about `AvroSerializationConfluentSchema` to Flink dist? If we move this serialization code in Flink dist we can drop `kafka-avro` dependencies from here. but still if you think using `String` is ok I can update the PR and use `String` instead of `AvroSerializationConfluentSchema` ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/6083 Why don't we output the events using the `SimpleStringSchema` without verification of the schema. That way we would no longer need `AvroSerializationConfluentSchema` and could get rid of the `kafka-avro` dependency. What do you think @medcv? ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann I made the changes and used `ConfluentRegistryAvroDeserializationSchema` for the Deserializations. I still using `AvroSerializationConfluentSchema` as we need to compare each income `Event` with `Schema` before sending the data to Kafka and use the schema registry concept to have a full end2end test. We might need to `AvroSerializationConfluentSchema` also to the Flink dist. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @dawidwys do u have an example using `ConfluentRegistryAvroDeserializationSchema`. I have some difficulties to make it work ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann PR is updated please review! I will also look into `ConfluentRegistryAvroDeserializationSchema` ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/6083 I agree with @dawidwys. We should use `ConfluentRegistryAvroDeserializationSchema` for the test. It is ok that this end-to-end test would then only go to Flink `1.6`. The benefit is that we only need to address the test related comments. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @dawidwys Thanks for the review! I will look into that PR. This test is for Release 1.4 and 1.5 and provide a proof that Flink V4.0 and V5.0 can work with ConfluentSchema Registry properly. As PR you mentioned will go to next release I think this test still would be useful the way it is and for sure we can update it later. @tillrohrmann Any thoughts? ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/6083 Just my two cents, we've recently introduced a `DeserializationSchema` that uses Confluent Schema Registry to decode data. Could you reuse it rather than introduce a new one. The PR in which it was introduced is here: #5995 ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann Thanks for the review! I will go through them and will make the changes shortly. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user dmpour23 commented on the issue: https://github.com/apache/flink/pull/6083 @medcv Regarding KeyedDeserialization/KeyedSerializationSchema. I assume that the blocking part is the casting in deserializeKey, serializeKey, ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @dmpour23 Thanks! I updated the import as`*.util.serialization` was Deprecated! ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user dmpour23 commented on the issue: https://github.com/apache/flink/pull/6083 Hi would this work for KeyedDeserialization/KeyedSerializationSchema. Should the import be: > org.apache.flink.api.common.serialization.DeserializationSchema ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann Thanks a lot for doing the review! ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/6083 Thanks for opening the PR @medcv. I will try to take a look next week. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann I would appreciate if you review or assign a reviewer to this PR. ---
[GitHub] flink issue #6083: [FLINK-8983] End-to-end test: Confluent schema registry
Github user medcv commented on the issue: https://github.com/apache/flink/pull/6083 @tillrohrmann please review ---
