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/03 03:15:24 UTC

[GitHub] [pulsar] coderzc opened a new pull request, #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

coderzc opened a new pull request, #15915:
URL: https://github.com/apache/pulsar/pull/15915

   Fixes #15899
   
   ### Motivation
   
   Now, don‘t register logical type conversions when use `SchemaDefinition.<T>builder().withJsonDef()`
   
   ### Modifications
   
   Register logical type conversions when use `SchemaDefinition.<T>builder().withJsonDef()`
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   *(Please pick either of the following options)*
   
   This change added tests and can be verified
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
     
   - [x] `doc-not-needed` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)


-- 
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] mattisonchao commented on pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1150900919

   Hi @coderzc 
   
   We re-run the test many times, but the test still fails. Could you take a look?


-- 
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] mattisonchao commented on pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1150877199

   /pulsarbot run-failure-checks


-- 
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 pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1151332399

   /pulsarbot run-failure-checks


-- 
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 pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1152575320

   /pulsarbot run-failure-checks


-- 
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 pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1147331110

   > PR
   
   What means?
   This PR modifies `SchemaUtil` rather than `StructSchema`.


-- 
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] codelipenghui commented on a diff in pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#discussion_r895164290


##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinitionBuilder.java:
##########
@@ -80,6 +80,15 @@
      */
     SchemaDefinitionBuilder<T> withPojo(Class pojo);
 
+    /**
+     * Set schema of pojo classLoader.
+     *
+     * @param classLoader pojo classLoader
+     *
+     * @return schema definition builder
+     */
+    SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader);

Review Comment:
   If need to add a public API to fix the issue, it should start with a proposal first.



-- 
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 pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1150958997

   > Hi @coderzc 
   > 
   > 
   > 
   > We re-run the test many times, but the test still fails. Could you take a look?
   
   ok


-- 
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 pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1151042433

   /pulsarbot run-failure-checks


-- 
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 pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1164077839

   @congbobo184 @mattisonchao PTAL


-- 
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 pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1152526327

   /pulsarbot run-failure-checks


-- 
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 a diff in pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
coderzc commented on code in PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#discussion_r896854052


##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinitionBuilder.java:
##########
@@ -80,6 +80,15 @@
      */
     SchemaDefinitionBuilder<T> withPojo(Class pojo);
 
+    /**
+     * Set schema of pojo classLoader.
+     *
+     * @param classLoader pojo classLoader
+     *
+     * @return schema definition builder
+     */
+    SchemaDefinitionBuilder<T> withClassLoader(ClassLoader classLoader);

Review Comment:
   OK, I have been submitted a PIP: 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] codelipenghui commented on pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1147192993

   @coderzc I have created a PR to clean up the code of StructSchema, the removed code is no longer useful when I reviewing this PR.


-- 
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 merged pull request #15915: [fix][client] Add classLoader field for `SchemaDefinition`

Posted by GitBox <gi...@apache.org>.
congbobo184 merged PR #15915:
URL: https://github.com/apache/pulsar/pull/15915


-- 
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] codelipenghui commented on pull request #15915: [fix][client] Register logical type conversions when use `SchemaDefinition.builder().withJsonDef()`

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15915:
URL: https://github.com/apache/pulsar/pull/15915#issuecomment-1150639661

   /pulsarbot run-failure-checks


-- 
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