You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/05/13 15:34:21 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #2106: Make LogReader more discoverable
jmark99 opened a new pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106
Use the KeywordExecutable service to make wal-info (the WAL LogReader class) available as part of the Accumulo command line. Can be used by running:
accumulo wal-info
Closes #2100
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631940460
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
The following code will print the message and then display the usage:
```
if (opts.files.isEmpty()) {
JCommander jc = new JCommander(opts);
jc.setProgramName("accumulo wal-info");
System.err.println("No WAL files were given");
jc.usage();
System.exit(1);
}
```
We can do this or merge the current changes and then place a follow-on ticket to implement Mike's suggestion.
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631950350
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
I will merge with the current update and we can create a follow-on ticket if we decide to pursue additional improvements.
--
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] [accumulo] ctubbsii commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631948568
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
As long as there's some way for users to discover the usage, I don't necessarily care about the details. I just wasn't sure if the current changes still supported some kind of discovery. If you say there is a help flag already, then that's sufficient for me. If you want to make additional improvements later, that's also fine.
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
<pre>
Usage: <main class> [options] <logfile> { <logfile> ...}
</pre>
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
Usage: <main class> [options] <logfile> { <logfile> ...}
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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] [accumulo] milleruntime commented on pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#issuecomment-840728126
Sounds good, thanks @jmark99
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
<pre>
Usage: <main class> [options] <logfile> { **<logfile** ...}
</pre>
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
`
Usage: <main class> [options] <logfile> { **<logfile** ...}
'
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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] [accumulo] ctubbsii commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631913046
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
This is okay, but I wonder if this will allow users to see the full usage. Is there a `--help` or `-?` option to show the usage, or if the command-line options are wrong, is there anyway for the user to see what the correct usage is? The code that this bit replaces did `new JCommander(opts).usage()`.
--
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] [accumulo] jmark99 merged pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 merged pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
<pre>
Usage: <main class> [options] <logfile> { **<logfile** ...}
</pre>
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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] [accumulo] milleruntime commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631931333
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
I think this is consistent with what we do with the other KeywordExecutable utilities. Perhaps we could add an exit method to the interface to use by them all. The method could take the Help object since it has an exit method that's called by the JCommander parseArgs.
--
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] [accumulo] jmark99 commented on a change in pull request #2106: Make LogReader more discoverable
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #2106:
URL: https://github.com/apache/accumulo/pull/2106#discussion_r631927019
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -73,19 +77,37 @@
* @param args
* - first argument is the file to print
*/
- public static void main(String[] args) throws IOException {
+ public static void main(String[] args) throws Exception {
+ new LogReader().execute(args);
+ }
+
+ @Override
+ public String keyword() {
+ return "wal-info";
+ }
+
+ @Override
+ public String description() {
+ return "Prints WAL Info";
+ }
+
+ @SuppressFBWarnings(value = "DM_EXIT",
+ justification = "System.exit is fine here because it's a utility class executed by a main()")
+ @Override
+ public void execute(String[] args) throws Exception {
Opts opts = new Opts();
- opts.parseArgs(LogReader.class.getName(), args);
+ opts.parseArgs("accumulo wal-info", args);
+ if (opts.files.isEmpty()) {
+ System.err.println("No WAL files were given");
+ System.exit(1);
+ }
Review comment:
There is a -h/--help and -? option available for the command. I followed the convention used with accumulo rfile-info. Initially I printed the given message and called the JCommander usage command. I wasn't particularly happy with what it printed out initially:
<pre>
Usage: <main class> [options] <logfile> { **<logfile** ...}
</pre>
I didn't find a way to have it replace '<main class>' with 'accumulo wal-info. The help options properly display 'accumulo wal-info ...' in the usage statement as desired. I'm okay putting the JCommander.usage back in but I wasn't sure if the <main class> verbiage would be confusing to users. Thoughts?
--
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