kevinrr888 commented on code in PR #5433:
URL: https://github.com/apache/accumulo/pull/5433#discussion_r2021630112
##########
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:
Ah I see. Yeah the concern is with the public API.
I'm still not entirely sure what should be done for this. Should probably
make changes to both 2.1 and 4.0. The main concerns are:
1) What ops are/aren't acceptable on system tables.
> in 2.1, can online/offline any table that's not the ROOT table, can
deleteRows of any table that's not the METADATA table, and can bulkImportv2 any
table that's not the ROOT table... Seems like these should be all system
tables, not just META or ROOT.
2)
> I'm wondering if it would be better to move all validation for the table
names (e.g., ensuring it's not a system table, ensuring the table exists,
doesn't exists, etc.) into TableOperationsImpl. Seems like some things are
validated in TableOperationsImpl and some are validated in FateServiceHandler
or both, which is a bit confusing. If we always validate in TableOperationsImpl
we can avoid the RPC.
If `FateServiceHandler.executeFateOperation` is called somewhere else other
than table operations, this may not work.
--
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
