ctubbsii commented on code in PR #5540:
URL: https://github.com/apache/accumulo/pull/5540#discussion_r2133351096
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1118,6 +1118,27 @@ public Map<String,String> modifyProperties(String
tableName,
}
}
+ /**
+ * Like modifyProperties(...), but if an AccumuloException is caused by a
TableNotFoundException,
+ * unwrap and rethrow the TNFE directly. This is a hacky, temporary
workaround that we can use
+ * until we are able to change the public API and throw TNFE directly from
all applicable methods.
+ */
+ private Map<String,String> modifyPropertiesUnwrapped(String tableName,
+ Consumer<Map<String,String>> mapMutator) throws TableNotFoundException,
AccumuloException,
+ AccumuloSecurityException, IllegalArgumentException {
+
+ try {
+ return modifyProperties(tableName, mapMutator);
+ } catch (AccumuloException ae) {
+ Throwable cause = ae.getCause();
+ if (cause instanceof TableNotFoundException) {
+ throw new
TableNotFoundException(context.getTableId(tableName).canonical(), tableName,
+ ae.getMessage(), ae);
Review Comment:
If the table is not found, then looking up the tableId this way is going to
throw a completely different TNFE, and the one you are trying to create here
will never actually get thrown.
Even if you replace the tableId with null instead of trying to look it up,
this construction will mangle the getMessage() on the exception quite a bit,
since the parameter here is supposed to be a description snippet that is used
to construct a larger message. But what you're passing in here is already a
full message from the previous exception.
I think you can just do:
```suggestion
throw new TableNotFoundException(null, tableName, null, ae);
```
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1118,6 +1118,27 @@ public Map<String,String> modifyProperties(String
tableName,
}
}
+ /**
+ * Like modifyProperties(...), but if an AccumuloException is caused by a
TableNotFoundException,
+ * unwrap and rethrow the TNFE directly. This is a hacky, temporary
workaround that we can use
+ * until we are able to change the public API and throw TNFE directly from
all applicable methods.
+ */
+ private Map<String,String> modifyPropertiesUnwrapped(String tableName,
+ Consumer<Map<String,String>> mapMutator) throws TableNotFoundException,
AccumuloException,
+ AccumuloSecurityException, IllegalArgumentException {
Review Comment:
Don't need to declare RTEs.
```suggestion
private Map<String,String> modifyPropertiesUnwrapped(String tableName,
Consumer<Map<String,String>> mapMutator) throws
TableNotFoundException, AccumuloException,
AccumuloSecurityException {
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
