[improve][admin] Pretty print bookies racks-placement command output #20516
Conversation
|
@vineeth1995 Please add the following content to your PR description and select a checkbox: |
github-actions
bot
added
doc-label-missing
doc-not-needed
| @@ -35,7 +38,8 @@ private class GetAll extends CliCommand { | |||
|
|
|||
| @Override | |||
| void run() throws Exception { | |||
| print(getAdmin().bookies().getBookiesRackInfo()); | |||
| Gson gson = new GsonBuilder().setPrettyPrinting().create(); | |||
| print(gson.toJson(getAdmin().bookies().getBookiesRackInfo())); | |||
There was a problem hiding this comment.
I found this call is already back by:
<T> void print(T item) {
try {
if (item instanceof String) {
System.out.println(item);
} else {
System.out.println(writer.writeValueAsString(item));
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}... where
private static ObjectMapper mapper = ObjectMapperFactory.create();
private static ObjectWriter writer = mapper.writerWithDefaultPrettyPrinter();Isn't the current code pretty print? Otherwise, we may try to fix the Jackson ObjectMapper config instead of wrapping up one more layer on Gson to hack.
There was a problem hiding this comment.
Hi @tisonkun, you are right but since BookiesRackConfiguration class extends TreeMap, it goes to the first definition of print
<K, V> void print(Map<K, V> items) { for (Map.Entry<K, V> entry : items.entrySet()) { print(entry.getKey() + " " + entry.getValue()); } }
where it is serialized to string already. So it does not do pretty print.
For example below is the output with current implementation:
default {127.0.0.1:3181=BookieInfoImpl(rack=/rack1, hostname=127.0.0.1)}
Output after the change:
{ "default": { "127.0.0.1:3181": { "rack": "/rack1", "hostname": "127.0.0.1" } }
|
@vineeth1995 here is a method you can make use of: diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CliCommand.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CliCommand.java
index 7a6836eb74..c96b0bd436 100644
--- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CliCommand.java
+++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CliCommand.java
@@ -214,13 +214,21 @@ public abstract class CliCommand {
if (item instanceof String) {
System.out.println(item);
} else {
- System.out.println(writer.writeValueAsString(item));
+ prettyPrint(item);
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}
+ <T> void prettyPrint(T item) {
+ try {
+ System.out.println(writer.writeValueAsString(item));
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
private static ObjectMapper mapper = ObjectMapperFactory.create();
private static ObjectWriter writer = mapper.writerWithDefaultPrettyPrinter();
private static Set<Character> sizeUnit = Sets.newHashSet('k', 'K', 'm', 'M', 'g', 'G', 't', 'T');
diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBookies.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBookies.java
index 8c0b4d80ae..27502a305a 100644
--- a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBookies.java
+++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBookies.java
@@ -23,9 +23,6 @@ import com.beust.jcommander.ParameterException;
import com.beust.jcommander.Parameters;
import com.google.common.base.Strings;
import java.util.function.Supplier;
-
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
import lombok.NonNull;
import org.apache.pulsar.client.admin.PulsarAdmin;
import org.apache.pulsar.common.policies.data.BookieInfo;
@@ -38,8 +35,7 @@ public class CmdBookies extends CmdBase {
@Override
void run() throws Exception {
- Gson gson = new GsonBuilder().setPrettyPrinting().create();
- System.out.println(gson.toJson(getAdmin().bookies().getBookiesRackInfo()));
+ prettyPrint(getAdmin().bookies().getBookiesRackInfo());
}
} |
|
I don't know why we treat String, List and Map specifically so I don't just print all types with pretty printer. But it should be simple to follow #20516 (comment) that we don't sometimes use Jackson and the other time use Gson. It's too casual and the hack can create more chaos. |
|
@tisonkun I made the requested changes. Can you please approve? |
Codecov Report
@@ Coverage Diff @@
## master #20516 +/- ##
=============================================
+ Coverage 36.78% 72.90% +36.11%
- Complexity 12059 31917 +19858
=============================================
Files 1690 1867 +177
Lines 129001 138565 +9564
Branches 14041 15219 +1178
=============================================
+ Hits 47453 101017 +53564
+ Misses 75294 29525 -45769
- Partials 6254 8023 +1769
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Merging... Thanks for your contribution @vineeth1995! |
Motivation
Pretty print the output from "bin/pulsar-admin bookies racks-placement" command so that it is in a more readable format
Modifications
Use Gson package to pretty print the output of the above command
Verifying this change
This change is already covered by existing tests, such as bookies() testcase in PulsarAdminToolTest.java
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-complete