> On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java, > > line 24 > > <https://reviews.apache.org/r/15166/diff/1/?file=376077#file376077line24> > > > > If we make TableNamespaceExistException extend AccumuloExtension we can > > get around a lot of api compatibility issues
That doesn't follow the existing pattern with TableExistsException, and the Exception does not appear to be thrown in any existing API. To what API-compatibility issues are you referring? > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > core/src/main/java/org/apache/accumulo/core/client/mock/MockSecurityOperations.java, > > line 135 > > <https://reviews.apache.org/r/15166/diff/1/?file=376093#file376093line135> > > > > This is also inconsistent with the public API In what way? I've changed it to SecurityErrorCode.Table_NAMESPACE_DOESNT_EXIST. Is that what you mean? > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableNamespaceOperations.java, > > line 181 > > <https://reviews.apache.org/r/15166/diff/1/?file=376096#file376096line181> > > > > Ticket should be made for this Created ACCUMULO-1865 > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > core/src/main/java/org/apache/accumulo/core/client/security/SecurityErrorCode.java, > > line 41 > > <https://reviews.apache.org/r/15166/diff/1/?file=376098#file376098line41> > > > > Doesn't there need to be a corresponding TABLE_NAMESPACE_ALREADY_EXISTS? No, there is no security-related error that can fail an operation because something exists. Those may still be errors, but not security errors. Likewise, there's no SecurityErrorCode.TABLE_ALREADY_EXISTS either. > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java, > > line 431 > > <https://reviews.apache.org/r/15166/diff/1/?file=376130#file376130line431> > > > > This error could cause confusion when you have a table in the default > > namespace with the same name as a namespace (if that's possible?) I don't think so, not since the "why" string explicitly says "Could not find table namespace while getting configuration." > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > server/base/src/main/java/org/apache/accumulo/server/conf/TableNamespaceConfWatcher.java, > > line 29 > > <https://reviews.apache.org/r/15166/diff/1/?file=376133#file376133line29> > > > > Why is this overriding the logging? Because TableConfWatcher does. If we don't need to do that, we can create a separate ticket. > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java, > > line 311 > > <https://reviews.apache.org/r/15166/diff/1/?file=376139#file376139line311> > > > > Part of is wondering we want to change the hasTablePermission(blah) to > > hit both the table and the namespace I'm a big fan of changing this whole pluggable permissions model to one where we have a simple: boolean canPerform(User user, Action action); // possibly with an optional "environment" as an arbitrary property map With a few defined Actions, like TableRenameAction(oldName, newName), etc. and User with a strict interface like "boolean isAuthenticated(); String getPrincipal();", the pluggable permissions handler becomes an operations-based implementation of *policy*, rather than a dumb storage for a difficult to extend set of permissions *tokens*. The default implementation will still be token based, but the API will be much cleaner, much more extensible/pluggable, and wouldn't change when we add new features like this. Had we done that, this would be trivial to extend in a sensible way. > On Nov. 1, 2013, 6:19 p.m., John Vines wrote: > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java, > > line 309 > > <https://reviews.apache.org/r/15166/diff/1/?file=376152#file376152line309> > > > > Locking on NamespaceIds for compactions, and many other FATE > > operations, could be a nasty bottleneck. Do we really need to lock on them?! I'm not sure. Probably not, but could there be problems with moving tables between namespaces in some cases if not? Or, perhaps preventing that would be a better way to go. - Christopher ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15166/#review28029 ----------------------------------------------------------- On Oct. 31, 2013, 10:01 p.m., Christopher Tubbs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15166/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2013, 10:01 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-802 > https://issues.apache.org/jira/browse/ACCUMULO-802 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-802 Tablespaces (Table Namespaces), work done by Sean Hickey, > https://github.com/Wisellama/accumulo/tree/ACCUMULO-802, rebase'd onto latest > master (including 210 commits) > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/Constants.java > 9db0c405c5b9fbac13fa735c3ffd6433e9831051 > core/src/main/java/org/apache/accumulo/core/client/Connector.java > bbfa55f4b9ad8fc0e5f0c0058e2e0564685d7c85 > > core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/TableNamespaceNotEmptyException.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/TableNamespaceNotFoundException.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperations.java > 86a3ff271fc2e085c426da3156cfab9cdbb5c36b > > core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperationsImpl.java > 0f0e998f334631cfb342f9a39fdd12e62aa98f13 > > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperations.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsHelper.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsImpl.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsImpl.java > 1b652f99e23e2dfa06c7373a6f4c3c044f5cd1a3 > core/src/main/java/org/apache/accumulo/core/client/impl/ConnectorImpl.java > bd1156912849b13ebad8f6b974fd35d8adf18f1d > > core/src/main/java/org/apache/accumulo/core/client/impl/TableNamespaces.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java > 8bc725a3405a79c20c690bff3f6fdb6f713be523 > > core/src/main/java/org/apache/accumulo/core/client/impl/thrift/ClientService.java > 488e0654720250542145e0d543ad16813f8d8b6d > > core/src/main/java/org/apache/accumulo/core/client/impl/thrift/SecurityErrorCode.java > b706ce87bb1e8a525e585b14b7fb8563c3493c2c > core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java > 5ee144d9eb67c8e79dad870abbd823cac64e111e > core/src/main/java/org/apache/accumulo/core/client/mock/MockConnector.java > 80ec5133334f9ef466dd7e4077230acfd2c77fb5 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockSecurityOperations.java > 765cda9cac6fb3bc7df9eb8db1c83846960e85c3 > core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java > 3dcab11bd50bf189ccc4cbaf97d3d4ecb9133047 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableNamespace.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableNamespaceOperations.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java > a3c2043b8120e13a190eb173d8342c205700382f > > core/src/main/java/org/apache/accumulo/core/client/security/SecurityErrorCode.java > fb51387fabea6d47854048daad4c8ec14047ab33 > > core/src/main/java/org/apache/accumulo/core/master/thrift/MasterClientService.java > 5b9949a1b6b7997d08de371c9a36c1e1a2b50b1f > core/src/main/java/org/apache/accumulo/core/security/SystemPermission.java > 699396e7e47b858ea8273c969f79dc7b3ebdc577 > > core/src/main/java/org/apache/accumulo/core/security/TableNamespacePermission.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java > 52e1d04998060954b7f09a3b9a6d9fdd898208b0 > core/src/main/java/org/apache/accumulo/core/util/shell/ShellCompletor.java > 85f68ef2464cb784861a587badd4835d842d8b87 > core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptions.java > 31529730935ff78de6c39fea531e20e2ade30a2d > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/CloneNamespaceCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/ConfigCommand.java > 73d21ef6108282c9dd8dd8bc0f2ce77524acedcf > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/ConstraintCommand.java > 4280555dfce0856e2e1329412b0051b15ce5a748 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateNamespaceCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateTableCommand.java > 0ad787d67eaa5a90d26e113d7a79f0f3218ce0f4 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/DUCommand.java > 189458a504dce012f3c579ff3b8425f030ad1b54 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java > d0644828af83f5c2d2ecbc58bd7545d958be31a0 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteNamespaceCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteTableCommand.java > 35c534d791648267539833b922643fc65dd600d4 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/GrantCommand.java > f3e720029714da63a7ec7e2bfb7587443ed7ed8b > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/ListIterCommand.java > e2d4d915c3311205ca4731b449769c37cd92ac82 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/NamespacePermissionsCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/NamespacesCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/OptUtil.java > f0c14d4cfd9ebf5a480ac63bb9dcd41e085b22f7 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/RenameNamespaceCommand.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/RenameTableCommand.java > 057f2b4374b3cc137a20158e70c33dd97c019c87 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/RevokeCommand.java > 676284aa9591d05a946d0bc4290ff88c3eb04eba > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java > 3a7155687994049d2580f7b39e0df1a39d52c385 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/TableOperation.java > 2bec526af38d328ea86cb21860471d8bed384eac > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/TablesCommand.java > 19b49e9cb4ca8483de55e644700d2a68d14ab42f > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/UserPermissionsCommand.java > 79d78da3b0f6de47f57740ee13678792b61a87c7 > core/src/main/thrift/client.thrift 2f5e9d191a0d80d32e2a88a6e31845f3b93ae289 > core/src/main/thrift/master.thrift 19960c8857295d22ffba728c95501f069ba953c0 > > core/src/test/java/org/apache/accumulo/core/client/mock/MockTableNamespacesTest.java > PRE-CREATION > server/base/src/main/java/org/apache/accumulo/server/ServerConstants.java > 9e0ac393deb962799e4e98753db1814dd630d14e > > server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java > 3f1aaa29ef023d2f33fa5b35aad2e57df9b6aff4 > > server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfiguration.java > c9fd5a141a437aef6f1a9d8f8c80999f19af6ec8 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java > 4c581530b77a88e141837507a32cdc4f89982c56 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableNamespaceConfWatcher.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/conf/TableNamespaceConfiguration.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/conf/TableParentConfiguration.java > PRE-CREATION > server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java > 0477a443e307e174965140f0fd14c898a0d98727 > > server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java > 3e0a2bf0e06227780adb92967d439212c0316924 > > server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java > ee337a5d23f04b321a306a306c41ea59e2f731a0 > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java > f00159cc21d8a7b8a47efbbc3d1f2bd96f479a03 > > server/base/src/main/java/org/apache/accumulo/server/security/handler/InsecurePermHandler.java > b57abfec483e3e07b85ed41148e11789fc45461c > > server/base/src/main/java/org/apache/accumulo/server/security/handler/PermissionHandler.java > 72c64b5897a4b0bf7de8081a76288fd8fb639021 > > server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java > f2196035d93ce5718b09e9754d130015515da91f > > server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKSecurityTool.java > 3b9d8b24b85996dfbf734d4910aa6cd0d9247beb > > server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java > 8a74d0b936d8fad3b020a230b6b20536d740288f > > server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java > cdee8fbe448592120a5cc5b6682aedfafca7e7bf > server/master/src/main/java/org/apache/accumulo/master/Master.java > 8a1a9b272bf332258febf46b7535d12f83a1dbd0 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CancelCompactions.java > dd4c2292996d758860191fc054631812adfa34b2 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ChangeTableState.java > 697c15e173b13bc5909862998b8742708bc933c1 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java > 8d906d602ae1abdd2f4d774c64b3b4cd1174aa72 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTableNamespace.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java > 160fc7e89320250d24e89526188528ff4786cd6d > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java > df2e0285b10fbdacb9fcacd007b48c1e2891d5d1 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTableNamespace.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java > 427125721cfaa122378bfe74b6c89b96b2f93137 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTableNamespace.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ExportTable.java > 22df3b340bbb9e76ef1fabe16b521654eb7a6bcb > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java > b91da522ff13b2be4de3fe82c47875e74cc1e569 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java > 3069aaf9eea3044d14bf7bb7b81c0270a0997f08 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTableNamespace.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/tableOps/TableRangeOp.java > 0ad2196a30a959f5ca5c7bdaf6d46a8628fd957a > server/master/src/main/java/org/apache/accumulo/master/tableOps/Utils.java > fa14f43100f5a13b16d2d34b95a8466501b93299 > server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java > 0ef6c6b867b284e86f8252ebb19913b3c1831362 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/ChangePermissions.java > be2de2a547aecc72eca7e2c980964ef85551aae8 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CheckPermission.java > 5fa9bc49439974071fe12d97b371eb4e6ac3db3f > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java > 65d3aed4faf1f2a360aa0d4df123390206eae0f3 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTableNamespace.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Config.java > b47f1a946e904820a0cc83a4b3e078d40f11bb99 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CreateTable.java > 0aa7f7ad0882a3d82bd6ca423a3a8a3dac706fba > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CreateTableNamespace.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/DeleteTableNamespace.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Merge.java > 8fcfab593071458954939970892a88b8793088b3 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/OfflineTableNamespace.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/RenameTable.java > d4a4e956699c2b2a483d6360e201da3eccd29847 > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/RenameTableNamespace.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java > 2cf37cecb8223b31b58e1afa4fa08ebf9967a8e1 > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/WalkingSecurity.java > 0ddb75254da156d75bbb0ca1f0a6e63c2443ab1c > test/src/test/java/org/apache/accumulo/test/ShellServerIT.java > 31867f39dd4d9661a00557c3291258b5e2507b30 > test/src/test/java/org/apache/accumulo/test/TableNamespacesIT.java > PRE-CREATION > test/src/test/java/org/apache/accumulo/test/functional/PermissionsIT.java > b8e1a4fcfcd1f02aa12926a44046493dbe74fe15 > test/system/randomwalk/conf/modules/Concurrent.xml > 03e5542d6b5b02f09e899301c68cf1d6746285f5 > > Diff: https://reviews.apache.org/r/15166/diff/ > > > Testing > ------- > > > Thanks, > > Christopher Tubbs > >
