DomGarguilo commented on code in PR #5433:
URL: https://github.com/apache/accumulo/pull/5433#discussion_r2018852926
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1511,15 +1511,13 @@ private void changeTableState(String tableName, boolean
wait, TableState newStat
switch (newState) {
case OFFLINE:
op = TFateOperation.TABLE_OFFLINE;
- if (tableName.equals(AccumuloTable.METADATA.tableName())
- || tableName.equals(AccumuloTable.ROOT.tableName())) {
- throw new AccumuloException("Cannot set table to offline state");
- }
+ NOT_BUILTIN_TABLE.validate(tableName);
Review Comment:
I see a similar pattern to what was replaced in this PR in `exists()` where
just the medata and root tables are treated differently:
https://github.com/apache/accumulo/blob/0480adf94116e2e98f5d64e9df3a1b8d8ef6e6a1/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java#L223-L229
I am not sure if all system tables should be handled in the same way here or
not.
On a side note for the code above, it seems like the call to the validator
and the early return blocks could be reversed so that we return early and avoid
the validator check if its one of those tables. Not sure of the correctness of
this either but wanted to point it out just in case.
--
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]
