You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/14 13:53:54 UTC

[GitHub] [pulsar] coderzc opened a new issue, #16058: PIP-177: Add the classLoader field for SchemaDefinition

coderzc opened a new issue, #16058:
URL: https://github.com/apache/pulsar/issues/16058

   ## Motivation
   
   Now, don‘t register logical type conversions when use `SchemaDefinition.<T>builder().withJsonDef()` to create the schema, beacase it without classLoader param.
   
   See: https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java#L58-L68
   
   We can add the classLoader field for SchemaDefinition, user can manually pass a classLoader to register logical type conversions
   
   ## Goal
   
   This proposes to add the classLoader field for SchemaDefinition. When using `SchemaDefinition.<T>builder().withJsonDef()` to create the schema it must manually specify a classLoader otherwise, the converter will not work.
   
   The priority of the classLoader field will be higher than by the `pojoClass.getClassLoader()`.
   
   ## API Changes
   
   ```java
   public class SchemaDefinition {
       //....
   
       /**
        * Set schema of pojo classLoader.
        *
        * @param classLoader pojo classLoader
        *
        * @return schema definition builder
        */
       SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader);
   }
   ```
   
   ```java
   public class SchemaDefinition {
       //....
   
       /**
        * Get pojo classLoader.
        *
        * @return pojo schema
        */
       ClassLoader getClassLoader();
   }
   ```
   
   ## Implementation
   
   Add the classloader field for SchemaDefinition.
   


-- 
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: commits-unsubscribe@pulsar.apache.org.apache.org

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

[GitHub] [pulsar] coderzc closed issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
coderzc closed issue #16058: PIP-177: Add the classLoader field for SchemaDefinition
URL: https://github.com/apache/pulsar/issues/16058


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] eolivelli commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1155600888

   how does this work with other Schema types ?


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] eolivelli commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1157454917

   LGTM thanks


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] congbobo184 commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1161088628

   +1 (none-binding)  Thanks, 
   Bo
   
   > 2022年6月14日 下午9:54，Cong Zhao ***@***.***> 写道：
   > 
   > 
   > Motivation
   > 
   > Now, don‘t register logical type conversions when use SchemaDefinition.<T>builder().withJsonDef() to create the schema, beacase it without classLoader param.
   > 
   > See: https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java#L58-L68 <https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/reader/AvroReader.java#L58-L68>
   > We can add the classLoader field for SchemaDefinition, user can manually pass a classLoader to register logical type conversions
   > 
   > Goal
   > 
   > This proposes to add the classLoader field for SchemaDefinition. When using SchemaDefinition.<T>builder().withJsonDef() to create the schema it must manually specify a classLoader otherwise, the converter will not work.
   > 
   > The priority of the classLoader field will be higher than by the pojoClass.getClassLoader().
   > 
   > API Changes
   > 
   > public class SchemaDefinition {
   >     //....
   > 
   >     /**
   >      * Set schema of pojo classLoader.
   >      *
   >      * @param classLoader pojo classLoader
   >      *
   >      * @return schema definition builder
   >      */
   >     SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader);
   > }
   > public class SchemaDefinition {
   >     //....
   > 
   >     /**
   >      * Get pojo classLoader.
   >      *
   >      * @return pojo schema
   >      */
   >     ClassLoader getClassLoader();
   > }
   > Implementation
   > 
   > Add the classloader field for SchemaDefinition.
   > 
   > —
   > Reply to this email directly, view it on GitHub <https://github.com/apache/pulsar/issues/16058>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJKEXQV6UUX2KA4HKYBUUDLVPCFJFANCNFSM5YXZWOKA>.
   > You are receiving this because you are subscribed to this thread.
   > 
   
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] coderzc commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
coderzc commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1157405423

   @eolivelli Do you have any questions about this proposal?


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] coderzc commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
coderzc commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1158458641

   @codelipenghui @congbobo184 @eolivelli @mattisonchao @shibd I sent a PIP vote mail, take a look at it.
   mail: https://lists.apache.org/thread/ydslz3h8ymjjwm5ng02kwszmc5j5hy30


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] coderzc commented on issue #16058: PIP-177: Add the classLoader field for SchemaDefinition

Posted by GitBox <gi...@apache.org>.
coderzc commented on issue #16058:
URL: https://github.com/apache/pulsar/issues/16058#issuecomment-1155926899

   > how does this work with other Schema types ?
   
   @eolivelli it only work with the avro schema


-- 
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: commits-unsubscribe@pulsar.apache.org

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