kevinrr888 commented on code in PR #5510:
URL: https://github.com/apache/accumulo/pull/5510#discussion_r2063662674
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -2243,7 +2241,6 @@ private void validatePropertiesToSet(Map<String,String>
opts, Map<String,String>
public void setTabletAvailability(String tableName, Range range,
TabletAvailability availability)
throws AccumuloSecurityException, AccumuloException {
EXISTING_TABLE_NAME.validate(tableName);
- NOT_BUILTIN_TABLE.validate(tableName);
Review Comment:
In that case we get an `IllegalArgumentException` for a built in table here
instead of a `AccumuloException`. I agree that there is no need for the RPC
beforehand, but this is the pattern taken for all the other methods:
`IllegalArgumentException` - The table name doesn't make sense. Not expected
format
`AccumuloException` - The table operation cannot be performed on that table
And was probably the original intention with
https://github.com/apache/accumulo/issues/1953
and I think it should be consistent across all operations. If we want to
keep built in table here, we should have it everywhere applicable on client-side
--
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]
