[
https://issues.apache.org/jira/browse/HADOOP-10736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14046274#comment-14046274
]
Mike Yoder commented on HADOOP-10736:
-------------------------------------
{quote}
KeyProvider.java:
Use final on StringBuilder decl.
s/append( MessageFormat/append(MessageFormat/
s/) )/))/
s/)-1)/) - 1)/
If I'm reading toString() correctly, it looks like it will be
[foo=bar],[baz=quux],[k=v]. IWBNI there was a space after the , so ], [.
{quote}
Done for all. But oh man old coding conventions die hard. I'm used to putting
spaces around my )'s!
{quote}
KeyShell.java:
init(): Map<> attributes could use a final decl. Ditto attrval, attr, and val.
s/( /(/ and s/ )/)/
{quote}
Done.
{quote}
If I'm reading the code correctly, I think the usage message should be:
"\nAttributes must be in attributes=value form\n", since I don't think you can
really have 'attr = value' in the shell unless you are expecting three
different args, right? You have this correct further down in the USAGE string.
Nit: I might change the message to be: "\nAttributes and values must be
specified as attr=val\n" (if you make this change, then also update the USAGE
and DESC strings below).
{quote}
Yeah, this is one of those shell things that the user would have to know
about... "attribute = value" is fine since it's one argument (and I trim() the
strings), but attribute = value is not since it's three. How about...
{noformat}
Attributes must be in attribute=value form, or quoted
like "attribute = value"
{noformat}
{quote}
TestKeyShell.java:
In the comment, should jceks provider be JCEKS provider? Yeah, I know that's a
real nit...
final on tmpDir decl. Ditto delArgs, listArgs, listArgsM, etc.
s/( /(/ and s/ )/)/
I guess while you're in the neighborhood, you might as well convert the import
static org.junit.Assert.* to a set of explicit import statics.
{quote}
Done for all.
{quote}
Consider using GenericTestUtils for matching the output.
{quote}
Hmmm. Had a look, thanks for the pointer. You're thinking of assertMatches()?
Maybe, but a regex match might be overkill. I'd have to escape my []'s. I
think I'll pass on this one.
Thanks! New patch coming soon...
> Add key attributes to the key shell
> -----------------------------------
>
> Key: HADOOP-10736
> URL: https://issues.apache.org/jira/browse/HADOOP-10736
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Affects Versions: 3.0.0
> Reporter: Mike Yoder
> Assignee: Mike Yoder
> Fix For: 3.0.0
>
> Attachments: HADOOP-10736.patch
>
>
> The recent work in HADOOP-10696 added attribute-value pairs to keys in the
> KMS. Now the key shell needs to be updated to set/get these attributes.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
