[GitHub] sijie commented on a change in pull request #2730: [schema] provide a flag to disable/enable schema validation on broker and change default bytes producer to use `AUTO_PRODUCE_BYTES`

[GitBox]

sijie commented on a change in pull request #2730: [schema] provide a flag to 
disable/enable schema validation on broker and change default bytes producer to 
use `AUTO_PRODUCE_BYTES`
URL: https://github.com/apache/pulsar/pull/2730#discussion_r223102771
 
 

 ##
 File path: 
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleSchemaTest.java
 ##
 @@ -131,9 +132,7 @@ public void newProducerWithoutSchemaOnTopicWithSchema() 
throws Exception {
 }
 
 try (Producer p = 
pulsarClient.newProducer().topic(topic).create()) {
-Assert.fail("Shouldn't be able to connect to a schema'd topic with 
no schema");
-} catch (PulsarClientException e) {
-
Assert.assertTrue(e.getMessage().contains("IncompatibleSchemaException"));
+p.send("junkdata".getBytes(UTF_8));
 
 Review comment:
   updated with the tests


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #2730: [schema] provide a flag to disable/enable schema validation on broker and change default bytes producer to use `AUTO_PRODUCE_BYTES`

[GitBox]

sijie commented on a change in pull request #2730: [schema] provide a flag to 
disable/enable schema validation on broker and change default bytes producer to 
use `AUTO_PRODUCE_BYTES`
URL: https://github.com/apache/pulsar/pull/2730#discussion_r223093861
 
 

 ##
 File path: 
pulsar-client-schema/src/main/java/org/apache/pulsar/client/api/Schema.java
 ##
 @@ -40,6 +40,22 @@
  */
 public interface Schema {
 
+/**
+ * Check if the message is a valid object for this schema.
+ *
+ * The implementation can choose what its most efficient approach to 
validate the schema.
+ * If the implementation doesn't provide it, it will attempt to use {@link 
#decode(byte[])}
+ * to see if this schema can decode this message or not as a validation 
mechanism to verify
+ * the bytes.
+ *
+ * @param message the messages to verify
+ * @return true if it is a valid message
+ * @throws SchemaSerializationException if it is not a valid message
+ */
+default void validate(byte[] message) {
 
 Review comment:
   it throws SchemaSerializationException, it aligns with what other methods 
are doing. it just makes `validate` to be reused in other methods like `encode` 
and `decode`. if we change it to return boolean or throw a checked exception, 
it requires more changes if validate is going to be used in other methods. 
   
   however I am not strong on this, if you think it should return a boolean or 
throw a checked exception, let me know, I can make the change.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #2730: [schema] provide a flag to disable/enable schema validation on broker and change default bytes producer to use `AUTO_PRODUCE_BYTES`

[GitBox]

sijie commented on a change in pull request #2730: [schema] provide a flag to 
disable/enable schema validation on broker and change default bytes producer to 
use `AUTO_PRODUCE_BYTES`
URL: https://github.com/apache/pulsar/pull/2730#discussion_r222901063
 
 

 ##
 File path: conf/broker.conf
 ##
 @@ -517,6 +517,13 @@ exposePublisherStats=true
 # The schema storage implementation used by this broker
 
schemaRegistryStorageClassName=org.apache.pulsar.broker.service.schema.BookkeeperSchemaStorageFactory
 
+# Enforce schema validation on following cases:
+#
+# - if a producer without a schema attempts to produce to a topic with schema, 
the producer will be
+#   failed to connect. PLEASE be carefully on using this, since non-java 
clients don't support schema.
+#   if you enable this setting, it will cause non-java clients failed to 
produce.
+isSchemaValidationEnforced=false
 
 Review comment:
   yes agreed, such api should be controlled at namespace level. the change 
here is more providing a flag for smooth upgrading story. the flag is more for 
the people who operates the whole pulsar cluster, and the namespace level is 
for tenants. to me, namespace level control is a feature to provide fine 
granualarity on management, the flag here is make sure the shema enforcement 
can be enabled in a BC way when people upgrade a pulsar cluster.
   
   does that make sense to you?


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:
us...@infra.apache.org


With regards,
Apache Git Services