> On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/Authorizations.java, > > lines 101-107 > > <https://reviews.apache.org/r/16305/diff/1/?file=398441#file398441line101> > > > > Does this (or any of the "already in UTF-8 bytes" methods) validate > > that the bytes are actually UTF-8? If not, I would include a warning.
It doesn't, and it apparently doesn't matter upon construction, which makes a warning a very good idea, as invalid strings could cause problems much later. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java, > > lines 435-439 > > <https://reviews.apache.org/r/16305/diff/1/?file=398442#file398442line435> > > > > I think you'd be in rights to fix all the within-javadocs whitespace > > errors, like this one. Sure. I think this is one spot where there just happened not to be a whitespace error, within an existing Javadoc comment. I generally avoid futzing with those if I can, to reduce the changeset. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/Authorizations.java, > > line 222 > > <https://reviews.apache.org/r/16305/diff/1/?file=398441#file398441line222> > > > > Can we get a TODO or a follow on Jira that this shouldn't be making a > > copy? I'll file a follow-on JIRA. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java, > > line 26 > > <https://reviews.apache.org/r/16305/diff/1/?file=398444#file398444line26> > > > > Can we make this language stronger? maybe use RFC MAY, MUST, and MUST > > NOT language? Existing comment, but I'll strengthen it. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/SystemPermission.java, > > line 27 > > <https://reviews.apache.org/r/16305/diff/1/?file=398445#file398445line27> > > > > Can we make this language stronger? maybe use RFC MAY, MUST, and MUST > > NOT language? Existing comment, but I'll strengthen it. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/TablePermission.java, > > line 26 > > <https://reviews.apache.org/r/16305/diff/1/?file=398446#file398446line26> > > > > Can we make this language stronger? maybe use RFC MAY, MUST, and MUST > > NOT language? Existing comment, but I'll strengthen it. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java, > > lines 27-29 > > <https://reviews.apache.org/r/16305/diff/1/?file=398444#file398444line27> > > > > Can you add a brief description of the permissions, or are they > > documented somewhere else we can link to? I haven't found docs, and I don't think I could write anything particularly elucidating. I will file a follow-on JIRA. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/SystemPermission.java, > > lines 28-30 > > <https://reviews.apache.org/r/16305/diff/1/?file=398445#file398445line28> > > > > Can you add a brief description of the permissions, or are they > > documented somewhere else we can link to? I haven't found docs, and I don't think I could write anything particularly elucidating. I will file a follow-on JIRA. > On Dec. 16, 2013, 9:06 p.m., Sean Busbey wrote: > > core/src/main/java/org/apache/accumulo/core/security/TablePermission.java, > > lines 27-29 > > <https://reviews.apache.org/r/16305/diff/1/?file=398446#file398446line27> > > > > Can you add a brief description of the permissions, or are they > > documented somewhere else we can link to? I haven't found docs, and I don't think I could write anything particularly elucidating. I will file a follow-on JIRA. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16305/#review30513 ----------------------------------------------------------- On Dec. 16, 2013, 3:22 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16305/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2013, 3:22 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2030 > https://issues.apache.org/jira/browse/ACCUMULO-2030 > > > Repository: accumulo > > > Description > ------- > > Javadoc for o.a.a.core.security. > > I'll wait to submit the patch until a committer +1 or three business days > inactivity, whichever happens first. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/security/AuthorizationContainer.java > 041bf3c > core/src/main/java/org/apache/accumulo/core/security/Authorizations.java > 5cecefb > core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java > bd16b84 > core/src/main/java/org/apache/accumulo/core/security/Credentials.java > 45708a8 > > core/src/main/java/org/apache/accumulo/core/security/NamespacePermission.java > fb6847b > core/src/main/java/org/apache/accumulo/core/security/SystemPermission.java > 6bdb8e6 > core/src/main/java/org/apache/accumulo/core/security/TablePermission.java > 912b5fb > > core/src/main/java/org/apache/accumulo/core/security/VisibilityConstraint.java > ae29cc7 > > core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java > 45b595f > > core/src/main/java/org/apache/accumulo/core/security/VisibilityParseException.java > 2f46dc9 > > Diff: https://reviews.apache.org/r/16305/diff/ > > > Testing > ------- > > mvn javadoc:javadoc returns no errors or warnings for these classes. > > > Thanks, > > Bill Havanki > >
