> On Jan. 8, 2015, 6 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java, > > line 394 > > <https://reviews.apache.org/r/29701/diff/1/?file=812557#file812557line394> > > > > Would be nice to provide more detail to the user as this will be > > confusing. Explain what the credentials are any why they are needed as > > well as how to set them. > > Robert Levas wrote: > Interesting, but that maybe a lot of text. Also, the UI will be using > this value to determine if the credentials are set or invalid - at least > until we create a way to push an error code along with the payload.
You could use the existing msg as the prefix and the UI could simply do a startsWith() or contains() for now. There is already precendence for more details error messages. For example, for BP topology validtion errors, the message states all missing components. Also, all required properties are specified along with their config type. See https://issues.apache.org/jira/browse/AMBARI-5690 and https://issues.apache.org/jira/browse/AMBARI-6015 as examples. You are right that we don't want to write a novel, but we should try to be a helpful as we can without going overboard. As you mentioned, supporting error codes is the ideal solution. > On Jan. 8, 2015, 6 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java, > > line 411 > > <https://reviews.apache.org/r/29701/diff/1/?file=812557#file812557line411> > > > > Is AmbariException only thrown for invalid credentials? Would be very > > frustrating to report back bad credentials when there was actually another > > issue. See earlier comment regarding a more detailed msg. > > Robert Levas wrote: > An AmbariException is thrown when there is an issue testing the > administrator credentials. One of several reasons might generate this... > 1. Failure to execute kadmin > 2. Failure to communicate with the KDC via kdamin > 3. Failure to authenticate with the KDC via kadmin > > In all cases, something is technically wrong with the administrator > credentials since they have not been validated My concern is that if you tell the user the kredentials are invalid when we are not able to commmunicate with the KDC server or some other underlying issue occurs, it would be very confusing for the user. The current messsage basically says the credentials that are set are invalid. This is very different than 'unable to autheticate admin credentials: KDC server is unreachable'. > On Jan. 8, 2015, 6 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java, > > line 226 > > <https://reviews.apache.org/r/29701/diff/1/?file=812558#file812558line226> > > > > Since principalExists() also throws AmbariException but for other > > reasons (true error condition?) should this case be reported back via a > > different exception or mechanism? > > Robert Levas wrote: > It could be, should I create a KerberosOperationException? Any maybe > subclasses of that? InvalidAdministratorCredentialsExcetion, > MissingAdministratorCredentialsExcecption, etc... See above comment. My concern is that we provide accurate information to the user and not use the same msg for all failures/erros that occur while trying to validate. I will let you make the decision on how to do this. Basically, ask yourself whether the user would know how to react based on the response that is sent back. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29701/#review67218 ----------------------------------------------------------- On Jan. 8, 2015, 4:29 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29701/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2015, 4:29 a.m.) > > > Review request for Ambari, Jaimin Jetly, John Speidel, Nate Cole, and Robert > Nettleton. > > > Bugs: AMBARI-9014 > https://issues.apache.org/jira/browse/AMBARI-9014 > > > Repository: ambari > > > Description > ------- > > Provide the standard error code that will be returned along with the error > message. > > If administrative credentials are not available > { > "status" : 400, > "message" : "java.lang.IllegalArgumentException: Missing KDC administrator > credentials" > } > > If administrative credentials are not valid, for example, incorrect principal > or password (or keytab) > { > "status" : 400, > "message" : "java.lang.IllegalArgumentException: Invalid KDC administrator > credentials" > } > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 9662669 > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java > 0533228 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java > ae2d4b2 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java > 30e3c35 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java > a99628c > > ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java > 8f39f21 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java > 4c7e49d > > Diff: https://reviews.apache.org/r/29701/diff/ > > > Testing > ------- > > Manaully tested in test cluster > > #Jenkins Test Results > > Running org.apache.ambari.server.controller.KerberosHelperTest > Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.632 sec > > Running org.apache.ambari.server.stack.KerberosDescriptorTest > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.19 sec > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 01:00 h > [INFO] Finished at: 2015-01-08T04:22:16+00:00 > [INFO] Final Memory: 44M/513M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Robert Levas > >
