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 2020/09/27 14:31:25 UTC

[GitHub] [pulsar] 315157973 opened a new pull request #8143: [ISSUE 6408] Fix NumberFormatException

315157973 opened a new pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143


   
   Fixes #6408
   
   ### Motivation
   1)ExecutionException cannot be cast to org.apache.pulsar.client.admin.PulsarAdminException
   2)Client cannot recognize multiple addresses
   
   ### Modifications
   support multiple addresses
   
   
   ### Verifying this change
   PulsarFunctionPublishTest#testMultipleAddress


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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-699700635


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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-701987333


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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702118920


   > @315157973, waiting that this is reviewed and accepted, could you provide the "patched" jar files so that I can try to see if it helps?
   > I'm kinda stuck trying to create a function in my cluster with these "cannot be cast to org.apache.pulsar.client.admin.PulsarAdminException"
   > That would help me a lot, thanks!
   
   Only 3 files have been modified, you can copy them directly to your local.
   If you want to reproduce the above problem, roll back lines 194-198 of `PulsarAdmin.java`, and then execute the unit test `PulsarFunctionPublishTest#testMultipleAddress`.


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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702053748


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

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



[GitHub] [pulsar] rdehouss commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
rdehouss commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702122031


   I use pulsar in the docker and the jar files present contains only .class files so I cannot even update the files directly in the jar files.


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

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



[GitHub] [pulsar] 315157973 commented on a change in pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#discussion_r502755632



##########
File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java
##########
@@ -189,7 +191,11 @@ public PulsarAdmin(String serviceUrl,
         this.client = clientBuilder.build();
 
         this.serviceUrl = serviceUrl;
-        root = client.target(serviceUrl);
+        ServiceURI serviceUri = ServiceURI.create(serviceUrl);
+        root = client.target(String.format("%s://%s"
+                , serviceUri.getServiceScheme()
+                , serviceUri.getServiceHosts()[ThreadLocalRandom.current()
+                        .nextInt(serviceUri.getServiceHosts().length)]));
 

Review comment:
       > I have a little doubt. Is this fixed the problem that the pulsar function does not support multi-service URLs?
   
   Yes, when we create a Function, the Client initiates a request. Since multi-service URLs are not supported here, a `NumberFormatException` will be thrown.
   When you restore this code, then update the jar to the lib, create a Function with cli, you can reproduce the previous bug.




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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-699720177


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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702097427


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

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



[GitHub] [pulsar] jianyun8023 commented on a change in pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
jianyun8023 commented on a change in pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#discussion_r502753616



##########
File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java
##########
@@ -189,7 +191,11 @@ public PulsarAdmin(String serviceUrl,
         this.client = clientBuilder.build();
 
         this.serviceUrl = serviceUrl;
-        root = client.target(serviceUrl);
+        ServiceURI serviceUri = ServiceURI.create(serviceUrl);
+        root = client.target(String.format("%s://%s"
+                , serviceUri.getServiceScheme()
+                , serviceUri.getServiceHosts()[ThreadLocalRandom.current()
+                        .nextInt(serviceUri.getServiceHosts().length)]));
 

Review comment:
       I have a little doubt. Is this fixed the problem that the pulsar function does not support multi-service URLs?




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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702080047


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

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



[GitHub] [pulsar] sijie commented on a change in pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#discussion_r498000891



##########
File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/FunctionsImpl.java
##########
@@ -801,7 +859,10 @@ private void downloadFile(String destinationPath, WebTarget target) throws Pulsa
         try {
             downloadFileAsync(destinationPath, target).get(this.readTimeoutMs, TimeUnit.MILLISECONDS);
         } catch (ExecutionException e) {
-            throw (PulsarAdminException) e.getCause();
+            if (e.getCause() instanceof PulsarAdminException) {

Review comment:
       Can you abstract this into one common function?




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

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



[GitHub] [pulsar] 315157973 commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-699656970


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

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



[GitHub] [pulsar] rdehouss commented on pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
rdehouss commented on pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143#issuecomment-702113446


   @315157973, waiting that this is reviewed and accepted, could you provide the "patched" jar files so that I can try to see if it helps?
   I'm kinda stuck trying to create a function in my cluster with these "cannot be cast to org.apache.pulsar.client.admin.PulsarAdminException"
   That would help me a lot, 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.

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



[GitHub] [pulsar] codelipenghui merged pull request #8143: [ISSUE 6408] Fix NumberFormatException

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #8143:
URL: https://github.com/apache/pulsar/pull/8143


   


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

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