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 2021/06/16 06:04:30 UTC

[GitHub] [pulsar] mattisonchao opened a new pull request #10934: Fixed #fixed incorrect use toString method.

mattisonchao opened a new pull request #10934:
URL: https://github.com/apache/pulsar/pull/10934


   Fixed #fixed incorrect use toString method.
   
   Motivation
   I found an incorrect use toString method. I think it's maybe a 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] mattisonchao commented on pull request #10934: Fixed #fixed incorrect use toString method.

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


   @sijie  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] eolivelli commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,7 +414,7 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
+                        System.out.println(Arrays.toString(Base64.getDecoder().decode(msg))); // print decode

Review comment:
       Arrays.toString of a byte[] is still not very useful most of the times
   
   in other commands in the pulsar-client we have an option to print byte[] in hex 
   https://github.com/apache/pulsar/blob/27156e4866298c44e83724eeb10a4ee67317433c/pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java#L100
   
   can you please implement the support for 'displayHex' ?




-- 
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] eolivelli commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,8 +425,9 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
-                    }catch(Exception e) {
+                        String output = interpretByteArray(displayHex, Base64.getDecoder().decode(msg));

Review comment:
       this is the core of the change.
   it is hidden inside the code reformat.
   
   can you please revert the code reformat ?




-- 
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 #10934: Fixed #fixed incorrect use toString method.

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


   /pulsarbot rerun-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] mattisonchao commented on pull request #10934: Fixed #fixed incorrect use toString method.

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


   @sijie  If you have time, can you help me review the code? : )


-- 
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] mattisonchao commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,8 +425,9 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
-                    }catch(Exception e) {
+                        String output = interpretByteArray(displayHex, Base64.getDecoder().decode(msg));

Review comment:
       sure, I will fix it.




-- 
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] sijie merged pull request #10934: Fixed #fixed incorrect use toString method.

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


   


-- 
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 #10934: Fixed #fixed incorrect use toString method.

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


   @sijie The return type of ``Base64.getDecoder().decode(msg)`` is ``byte[]``, calling toString will print the byte array address instead of the value.


-- 
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] mattisonchao commented on pull request #10934: Fixed #fixed incorrect use toString method.

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


   @eolivelli 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] mattisonchao commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,8 +425,9 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
-                    }catch(Exception e) {
+                        String output = interpretByteArray(displayHex, Base64.getDecoder().decode(msg));

Review comment:
       sure, I will fix it.




-- 
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 removed a comment on pull request #10934: Fixed #fixed incorrect use toString method.

Posted by GitBox <gi...@apache.org>.
mattisonchao removed a comment on pull request #10934:
URL: https://github.com/apache/pulsar/pull/10934#issuecomment-868966228






-- 
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] sijie merged pull request #10934: Fixed #fixed incorrect use toString method.

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


   


-- 
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 #10934: Fixed #fixed incorrect use toString method.

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


   /pulsarbot rerun-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] mattisonchao commented on pull request #10934: Fixed #fixed incorrect use toString method.

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


   Why is there no problem running the test locally?


-- 
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] mattisonchao commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,7 +414,7 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
+                        System.out.println(Arrays.toString(Base64.getDecoder().decode(msg))); // print decode

Review comment:
       Ok, I will do that : )




-- 
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] mattisonchao removed a comment on pull request #10934: Fixed #fixed incorrect use toString method.

Posted by GitBox <gi...@apache.org>.
mattisonchao removed a comment on pull request #10934:
URL: https://github.com/apache/pulsar/pull/10934#issuecomment-868973258


   /pulsarbot rerun-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] mattisonchao commented on a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,7 +414,7 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
+                        System.out.println(Arrays.toString(Base64.getDecoder().decode(msg))); // print decode

Review comment:
       I have added support for `displayHex`




-- 
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 a change in pull request #10934: Fixed #fixed incorrect use toString method.

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdConsume.java
##########
@@ -414,8 +425,9 @@ private int consumeFromWebSocket(String topic) {
                     LOG.debug("No message to consume after waiting for 5 seconds.");
                 } else {
                     try {
-                        System.out.println(Base64.getDecoder().decode(msg));
-                    }catch(Exception e) {
+                        String output = interpretByteArray(displayHex, Base64.getDecoder().decode(msg));

Review comment:
       this is the core of the change.
   it is hidden inside the code reformat.
   
   can you please revert the code reformat ?




-- 
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 pull request #10934: Fixed #fixed incorrect use toString method.

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


   @sijie now the patch is in better shape.
   Please take a look when you have time


-- 
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 #10934: Fixed #fixed incorrect use toString method.

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


   /pulsarbot rerun-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