Details
-
Bug
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
2.8.0
-
None
Description
andrew.wang had some follow-up code review comments from HADOOP-12942. Hence this issue.
Ping lmccay as well.
The comments:
Overall this looks okay, the only correctness question I have is about the difference in behavior when the pwfile doesn't exist.
The rest are all nits, would be nice to do these cleanups though.
File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java:
Line 147:
Could this be a static helper?Line 161: new
The javadoc says it returns null in this situation. This is also a difference from the implementation in the AbstractJKSP. Intentional?Line 175: private void locateKeystore() throws IOException {
static helper? for the construct*Path methods too?File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java:
Line 50: @VisibleForTesting public static final String NO_VALID_PROVIDERS =
FYI for the future, our coding style is to put annotations on their own separate line.File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java:
Line 326: private char[] locatePassword() throws IOException {
this method looks very similar to the one in JavaKeyStoreProvider, except the env var it looks for is different, is there potential for code reuse?Line 394: " o In the environment variable " +
Using a "*" is the usual way of doing a bullet point, e.g. markdown and wiki syntax.Line 399: "http://hadoop.apache.org/docs/current/hadoop-project-dist/" +
This link is not tied to a version, so could be inaccurate.
Attachments
Attachments
Issue Links
- relates to
-
HADOOP-12942 hadoop credential commands non-obviously use password of "none"
- Resolved