accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 16305: ACCUMULO-2030 - Javadoc for core/security
Date Tue, 17 Dec 2013 15:00:59 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message