Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo >> mail # dev >> Review Request 15166: ACCUMULO-802 Tablespaces


Copy link to this message
-
Re: Review Request 15166: ACCUMULO-802 Tablespaces


> 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>

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: