You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "Suvrat1629 (via GitHub)" <gi...@apache.org> on 2025/02/06 15:23:46 UTC

[PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Suvrat1629 opened a new pull request, #5310:
URL: https://github.com/apache/accumulo/pull/5310

   This PR enhances the accumulo ec-admin running command by adding a --format flag to support structured output in CSV and JSON formats. This allows for easier programmatic parsing of running compaction details.
   
   Changes Introduced:
       Added --format flag (json, csv, human [default]) to ec-admin running.
       Implemented structured output formatting for better readability and parsing.
       Updated runningCompactions() method to handle different output formats.
       Ensured compatibility with existing behavior by keeping table format as the default.


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "Suvrat1629 (via GitHub)" <gi...@apache.org>.
Suvrat1629 commented on PR #5310:
URL: https://github.com/apache/accumulo/pull/5310#issuecomment-2656230359

   Closing this pr and opening a new one targeting the 2.1 branch


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "Suvrat1629 (via GitHub)" <gi...@apache.org>.
Suvrat1629 commented on PR #5310:
URL: https://github.com/apache/accumulo/pull/5310#issuecomment-2655551228

   @ddanielr gentle ping! Could you please take a look :) Thank you!


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "Suvrat1629 (via GitHub)" <gi...@apache.org>.
Suvrat1629 commented on PR #5310:
URL: https://github.com/apache/accumulo/pull/5310#issuecomment-2641020654

   Well I ran the `mvn org.codehaus.mojo:build-helper-maven-plugin:rootlocation net.revelc.code.formatter:formatter-maven-plugin:format net.revelc.code:impsort-maven-plugin:sort org.apache.maven.plugins:maven-checkstyle-plugin:check` and commited the changes, what else do I need to do?


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "Suvrat1629 (via GitHub)" <gi...@apache.org>.
Suvrat1629 commented on PR #5310:
URL: https://github.com/apache/accumulo/pull/5310#issuecomment-2642812399

   @ddanielr 
   since I have already pushed the changes to the main branch how do i shift them to the 2.1 branch?


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5310:
URL: https://github.com/apache/accumulo/pull/5310#discussion_r1945000001


##########
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java:
##########
@@ -66,13 +69,16 @@ static class CancelCommand {
 
   @Parameters(commandDescription = "list the running compactions")
   static class RunningCommand {
-    @Parameter(names = {"-d", "--details"},
-        description = "display details about the running compactions")
+    @Parameter(names = { "-d", "--details" }, description = "display details about the running compactions")
     boolean details = false;
+
+    @Parameter(names = { "-f", "--format" }, description = "output format: json, csv (default: human-readable)")
+    String format = "human"; // Default format
   }
 
   @Parameters(commandDescription = "list all compactors in zookeeper")
-  static class ListCompactorsCommand {}

Review Comment:
   This will fail the build. 
   
   The accumulo build runs verify format and checkstyle checks and will fail if they aren't followed. 
   
   [Formatter Settings](https://github.com/apache/accumulo/blob/5dffcd6b57a4f9e237a6c76ad54113ec5707fab8/pom.xml#L848-L864)
   
   [Checkstyle Rules](https://github.com/apache/accumulo/blob/5dffcd6b57a4f9e237a6c76ad54113ec5707fab8/pom.xml#L1027-L1137) 
   
   To avoid build failures, you can run the build locally before pushing using `mvn clean verify -DskipITs` and then run `git status` to see what files have changes that need to be committed.
   
   If you don't want to wait for the whole verify run to complete, you can run the maven plugins directly
   `mvn org.codehaus.mojo:build-helper-maven-plugin:rootlocation net.revelc.code.formatter:formatter-maven-plugin:format net.revelc.code:impsort-maven-plugin:sort org.apache.maven.plugins:maven-checkstyle-plugin:check`
   
   Running them directly will not test your code for completeness, it will just perform the auto format steps and produce warning messages needed for fixing checkstyle issues. 
   
    



##########
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java:
##########
@@ -156,55 +162,97 @@ private void listCompactorsByQueue(ServerContext context) {
     if (compactors.isEmpty()) {
       System.out.println("No Compactors found.");
     } else {
-      Map<String,List<ServerId>> m = new TreeMap<>();
+      Map<String, List<ServerId>> m = new TreeMap<>();

Review Comment:
   Same format issue



##########
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java:
##########
@@ -156,55 +162,97 @@ private void listCompactorsByQueue(ServerContext context) {
     if (compactors.isEmpty()) {
       System.out.println("No Compactors found.");
     } else {
-      Map<String,List<ServerId>> m = new TreeMap<>();
+      Map<String, List<ServerId>> m = new TreeMap<>();
       compactors.forEach(csi -> {
         m.putIfAbsent(csi.getResourceGroup(), new ArrayList<>()).add(csi);
       });
       m.forEach((q, c) -> System.out.println(q + ": " + c));
     }
   }
 
-  private void runningCompactions(ServerContext context, boolean details) {
-    CompactionCoordinatorService.Client coordinatorClient = null;
-    TExternalCompactionList running;
-    try {
-      coordinatorClient = getCoordinatorClient(context);
-      running = coordinatorClient.getRunningCompactions(TraceUtil.traceInfo(), context.rpcCreds());
-      if (running == null) {
-        System.out.println("No running compactions found.");
-        return;
-      }
-      var ecidMap = running.getCompactions();
-      if (ecidMap == null) {
-        System.out.println("No running compactions found.");
-        return;
+  private void runningCompactions(ServerContext context, boolean details, String format) {
+  CompactionCoordinatorService.Client coordinatorClient = null;
+  TExternalCompactionList running;
+  
+  try {
+    coordinatorClient = getCoordinatorClient(context);
+    running = coordinatorClient.getRunningCompactions(TraceUtil.traceInfo(), context.rpcCreds());
+
+    if (running == null || running.getCompactions() == null) {

Review Comment:
   You could replace the second null check with `getCompactionsSize` instead of returning the map.
   ```suggestion
       if (running == null || running.getCompactionsSize() == 0) {
   ```
   



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Add --format flag to ec-admin running for structured output (CSV/JSON) [accumulo]

Posted by "Suvrat1629 (via GitHub)" <gi...@apache.org>.
Suvrat1629 closed pull request #5310: Add --format flag to ec-admin running for structured output (CSV/JSON)
URL: https://github.com/apache/accumulo/pull/5310


-- 
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: notifications-unsubscribe@accumulo.apache.org

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