ctubbsii commented on a change in pull request #2215:
URL: https://github.com/apache/accumulo/pull/2215#discussion_r681389046
##########
File path:
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
##########
@@ -149,6 +152,59 @@ boolean testClassLoad(final String className, final String
asTypeName)
*/
void waitForBalance() throws AccumuloException;
+ /**
+ * Throws an exception if a tablet server can not be contacted.
+ *
+ * @param args
+ * Command line arguments passed in from user command.
+ * @param secretOption
+ * Specified instance secret to use for commands.
+ *
+ * @since 2.1.0
+ */
+ boolean fateFail(List<String> args, String secretOption) throws
AccumuloException;
+
+ /**
+ * Throws an exception if a tablet server can not be contacted.
+ *
+ * @param args
+ * Command line arguments passed in from user command.
+ * @param secretOption
+ * Specified instance secret to use for commands.
+ *
+ * @since 2.1.0
+ */
+ boolean fateDelete(List<String> args, String secretOption) throws
AccumuloException;
+
+ /**
+ * Throws an exception if a tablet server can not be contacted.
+ *
+ * @param args
+ * Command line arguments passed in from user command.
+ * @param filterTxid
+ * Parsed transaction IDs for print filter.
+ * @param filterStatus
+ * Parsed TStatus for print filter.
+ * @param secretOption
+ * Specified instance secret to use for commands.
+ *
+ * @since 2.1.0
+ */
+ String fatePrint(List<String> args, Set<Long> filterTxid, EnumSet<TStatus>
filterStatus,
Review comment:
> > Instead of returning "String" with "Print" in the method name, this
could return a map of fate IDs to statuses, or some similar data structure.
>
> Is there a benefit you had in mind with this possible change?
Well, mainly just that APIs should return objects that can be
manipulated/read/transformed by calling code. It's not a console REPL where we
have only STDIN/STDOUT and have to parse STDOUT from a command. We can have
actual typed data. So, there's no reason to resort to "String" types to
represent complex information in the API.
> Currently, the returning string is the entire output from `Admin.print`
that then gets printed to the shell. Admin print handles all the formatting,
the only thing FateCommand has to do is print it to the shell.
Right, that's all that the shell needs to do... but if we're going through
the effort of creating a public API endpoint, we're not doing it just for the
shell. The shell can take a complex object returned from the public API and
format it into a string for printing. But other callers to this API endpoint
might want to do something different with the data. When creating a new public
API endpoint, we should try to consider the value it has to a generic caller of
the public API, and not just the single use case that inspired its creation.
> Instead of 'Print', I could change the name to "fateStatus" as I think
that more accurately describes the intended use of the function. It shows the
status of either all the txid's in operation or the specific one the user
requests.
The name is fine. It could return something that looks like
`Set<FateStatus>` or `Map<Txid,Status>` or similar.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
