-
Notifications
You must be signed in to change notification settings - Fork 454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --format flag to ec-admin running for structured output (CSV/JSON) #5332
base: 2.1
Are you sure you want to change the base?
Conversation
@kevinrr888 I hope this fixes the fix the issue with the commit history :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comments. I suggest running mvn clean verify -Psunny
before committing future changes. This will catch a lot of potential issues with new code.
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
…min.java Co-authored-by: Kevin Rathbun <[email protected]>
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
Concerns regarding build failures have been addressed. No further comments at this time
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
server/base/src/main/java/org/apache/accumulo/server/util/ECAdmin.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Dave Marion <[email protected]>
This looks pretty good. Have you tested this locally? |
@dlmarion No, I did not understand how to setup the development environment but I wanted to contribute so I decided to open a pr regardless. Sorry about that😅 |
@Suvrat1629 - I would suggest looking at https://github.com/apache/fluo-uno to get a development environment set up so that you can test this locally. |
I ran this command and got done with the set up. |
@Suvrat1629 - looking at the fluo-uno readme, it looks like you should be able to set the ACCUMULO_REPO environment variable in the uno.conf file and then fluo-uno will build Accumulo from your feature branch. See https://github.com/apache/fluo-uno/blob/main/conf/uno.conf#L46. Once everything is running, Accumulo should be running from the directory specified by the ACCUMULO_HOME variable (see https://github.com/apache/fluo-uno/blob/main/conf/uno.conf#L126). You should be able to run the |
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.
Fixes #5205