You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/08/02 13:04:05 UTC

[GitHub] [rocketmq-schema-registry] humkum opened a new pull request, #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

humkum opened a new pull request, #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25

   1. provide specific avro serde & generic avro serde
   2. change some  annotation message.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] ferrirW commented on a diff in pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
ferrirW commented on code in PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#discussion_r936183073


##########
example/src/main/java/org/apache/rocketmq/schema/registry/example/serde/SpecificAvroSerdeDemo.java:
##########
@@ -19,26 +19,28 @@
 
 import org.apache.rocketmq.schema.registry.client.SchemaRegistryClient;
 import org.apache.rocketmq.schema.registry.client.SchemaRegistryClientFactory;
-import org.apache.rocketmq.schema.registry.client.serializer.AvroDeserializer;
-import org.apache.rocketmq.schema.registry.client.serializer.AvroSerializer;
-import org.apache.rocketmq.schema.registry.client.serializer.Deserializer;
-import org.apache.rocketmq.schema.registry.client.serializer.Serializer;
+import org.apache.rocketmq.schema.registry.client.serde.SpecificAvroSerde;
 
-public class AvroSerdeDemo {
+import java.io.IOException;
+
+public class SpecificAvroSerdeDemo {
 
     public static void main(String[] args) {
 
         String baseUrl = "http://localhost:8080/schema-registry/v1";

Review Comment:
   Instead, mock a SchemaRegistry in a unit test.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] MatrixHB commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
MatrixHB commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203411734

   What is the relation between SpecificAvroSerde, GernericAvroSerde and AvroSerde?Up-down dependency, or parallel ?
   
   If SpecificAvroSerde and GernericAvroSerde are exposed to users and they are dependent on AvroSerde, I think AvroSerde should be moved to common module, to avoid user confusion. 
   If they are parallel, it is better to merge the Serde package and the Serializer package. Or I will feel a bit confused about which to choose.
   
   @humkum @ferrirW 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] MatrixHB commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
MatrixHB commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203433179

   > > What is the relation between SpecificAvroSerde, GernericAvroSerde and AvroSerde?Up-down dependency, or parallel ?
   > > If SpecificAvroSerde and GernericAvroSerde are exposed to users and they are dependent on AvroSerde, I think AvroSerde should be moved to common module, to avoid user confusion. If they are parallel, it is better to merge the Serde package and the Serializer package. Or I will feel a bit confused about which to choose.
   > > @humkum @ferrirW
   > 
   > I can't agree more with you, their relationship is an Up-down dependency, and I agree to move AvroSerde to common package
   
   I‘m sorry. I try to move but it's more unreasonable because AvroSerde is dependent on SchemaRegistryClient, so let's just keep this way.
   I guess you have learned kafka-schema-registry, so I agree with your current design.
   
   --------------------------
   _Is it better to make a "avro" package under the "serde" path, leave expansion room for JSON serde and Protobuf serde ?_
   BTW, you can take this into consideration.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] duhenglucky merged pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
duhenglucky merged PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] ShannonDing commented on a diff in pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
ShannonDing commented on code in PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#discussion_r937303121


##########
example/src/main/java/org/apache/rocketmq/schema/registry/example/serde/GenericAvroSerdeDemo.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.schema.registry.example.serde;
+
+import org.apache.avro.Schema;
+import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.GenericRecordBuilder;
+import org.apache.rocketmq.schema.registry.client.SchemaRegistryClient;
+import org.apache.rocketmq.schema.registry.client.SchemaRegistryClientFactory;
+import org.apache.rocketmq.schema.registry.client.config.AvroDeserializerConfig;
+import org.apache.rocketmq.schema.registry.client.serde.avro.GenericAvroSerde;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+public class GenericAvroSerdeDemo {
+
+    public static void main(String[] args) {
+
+        String baseUrl = "http://localhost:8080/schema-registry/v1";
+        SchemaRegistryClient schemaRegistryClient = SchemaRegistryClientFactory.newClient(baseUrl, null);
+
+        Schema schema = new Schema.Parser().parse("{\"type\":\"record\",\"name\":\"Charge\",\"namespace\":\"org.apache.rocketmq.schema.registry.example.serde\",\"fields\":[{\"name\":\"item\",\"type\":\"string\"},{\"name\":\"amount\",\"type\":\"double\"}]}");
+        GenericRecord record = new GenericRecordBuilder(schema)
+                .set("item", "generic")
+                .set("amount", 100.0)
+                .build();
+
+        try (GenericAvroSerde serde = new GenericAvroSerde(schemaRegistryClient)) {
+            //configure
+            Map<String, Object> configs = new HashMap<>();
+            configs.put(AvroDeserializerConfig.USE_GENERIC_DATUM_READER, true);
+            serde.configure(configs);
+
+            //serialize
+            byte[] bytes = serde.serializer().serialize("TopicTest", record);
+
+            //deserialize
+            GenericRecord record1 = serde.deserializer().deserialize("TopicTest", bytes);
+            System.out.println("the origin object after ser/de is " + record1);

Review Comment:
   print origin record to show if it has different from result record.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] codecov-commenter commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203078565

   # [Codecov](https://codecov.io/gh/apache/rocketmq-schema-registry/pull/25?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#25](https://codecov.io/gh/apache/rocketmq-schema-registry/pull/25?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (217cc5c) into [main](https://codecov.io/gh/apache/rocketmq-schema-registry/commit/e85fbf7afeb7d43694f7974828e51720a1ee0645?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e85fbf7) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   ```diff
   @@          Coverage Diff          @@
   ##            main     #25   +/-   ##
   =====================================
     Coverage   0.00%   0.00%           
   =====================================
     Files         16      16           
     Lines        367     367           
     Branches      16      16           
   =====================================
     Misses       367     367           
   ```
   
   
   
   Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] MatrixHB commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
MatrixHB commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203414513

   Is it better to make a "avro" package under the "serde" path, leave expansion room for JSON serde and Protobuf serde.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] humkum commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
humkum commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203444750

   > > > What is the relation between SpecificAvroSerde, GernericAvroSerde and AvroSerde?Up-down dependency, or parallel ?
   > > > If SpecificAvroSerde and GernericAvroSerde are exposed to users and they are dependent on AvroSerde, I think AvroSerde should be moved to common module, to avoid user confusion. If they are parallel, it is better to merge the Serde package and the Serializer package. Or I will feel a bit confused about which to choose.
   > > > @humkum @ferrirW
   > > 
   > > 
   > > I can't agree more with you, their relationship is an Up-down dependency, and I agree to move AvroSerde to common package
   > 
   > I‘m sorry. I try to move to common module but it's more unreasonable because AvroSerde is dependent on SchemaRegistryClient, so let's just keep this way. I guess you have learned kafka-schema-registry, so I agree with your current design.
   > 
   > _Is it better to make a "avro" package under the "serde" path, leave expansion room for JSON serde and Protobuf serde ?_ BTW, you can take this into consideration.
   
   Okay , it's a future plan.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq-schema-registry] humkum commented on pull request #25: refine avro serde type to SpecificAvroSerde and GernericAvroSerde

Posted by GitBox <gi...@apache.org>.
humkum commented on PR #25:
URL: https://github.com/apache/rocketmq-schema-registry/pull/25#issuecomment-1203419743

   > What is the relation between SpecificAvroSerde, GernericAvroSerde and AvroSerde?Up-down dependency, or parallel ?
   > 
   > If SpecificAvroSerde and GernericAvroSerde are exposed to users and they are dependent on AvroSerde, I think AvroSerde should be moved to common module, to avoid user confusion. If they are parallel, it is better to merge the Serde package and the Serializer package. Or I will feel a bit confused about which to choose.
   > 
   > @humkum @ferrirW
   
   I can't agree more with you, their relationship is an Up-down dependency, and I agree to move AvroSerde to common package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org