[
https://issues.apache.org/jira/browse/HADOOP-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16392402#comment-16392402
]
Xiao Chen commented on HADOOP-15234:
------------------------------------
Thanks all for the comments.
bq. Sorry for asking you to make 2-3 revisions for such a simple patch.
While I appreciate the friendliness, IMO there is nothing to be sorry about.
We're all trying to solve the problem in the best way. Even if the problem
appears simple, as it turned out here the possible solutions are many, and I
think it's common to iterate over a few patches - it's simply development. :)
bq. without unit tests
I'm okay here given this is a supportability improvement.
bq. Preconditions
Since here we're just doing a one-off at service startup time, which is a rare
operation and not performance critical, I'd vote for readability. Null check
and throw is fine by me too.
bq. should we throw in the implementation of
Maybe I misunderstood, please let me know if so.
The factory [depends
on|https://github.com/apache/hadoop/blob/113f401f41ee575cb303ceb647bc243108d93a04/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderFactory.java#L97]
the null check to create the correct provider out of all the factories loaded
by service loader. So throwing in one of them would not work.
It may be reasonable to throw instead of returning null in
{{KeyProviderFactory#get}}, but that class is {{InterfaceAudience.Public}}.
I do have 1 comment on the patch:
Can we add {{providerString}} to the message being thrown, so the exception is
more self-explaining?
> NPE when initializing KMSWebApp
> -------------------------------
>
> Key: HADOOP-15234
> URL: https://issues.apache.org/jira/browse/HADOOP-15234
> Project: Hadoop Common
> Issue Type: Bug
> Components: kms
> Reporter: Xiao Chen
> Assignee: fang zhenyi
> Priority: Major
> Attachments: HADOOP-15234.001.patch, HADOOP-15234.002.patch
>
>
> During KMS startup, if the {{keyProvider}} is null, it will NPE inside
> KeyProviderExtension.
> {noformat}
> java.lang.NullPointerException
> at
> org.apache.hadoop.crypto.key.KeyProviderExtension.<init>(KeyProviderExtension.java:43)
> at
> org.apache.hadoop.crypto.key.CachingKeyProvider.<init>(CachingKeyProvider.java:93)
> at
> org.apache.hadoop.crypto.key.kms.server.KMSWebApp.contextInitialized(KMSWebApp.java:170)
> {noformat}
> We're investigating the exact scenario that could lead to this, but the NPE
> and log around it can be improved.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
