> On Dec. 12, 2014, 4:37 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java, > > line 29 > > <https://reviews.apache.org/r/28944/diff/1/?file=789378#file789378line29> > > > > Just checking that this comment makes sense to you as it doesn't to me.
Copy/Paste issue... I forgot to go back and fix the javadoc after promoting the class to an abstract class. > On Dec. 12, 2014, 4:37 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java, > > line 62 > > <https://reviews.apache.org/r/28944/diff/1/?file=789378#file789378line62> > > > > just checking that this class doesn' need to be thread safe Nope... the expectation is that this class is used all in one shot - create, open, write, close, destroy > On Dec. 12, 2014, 4:37 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java, > > line 67 > > <https://reviews.apache.org/r/28944/diff/1/?file=789379#file789379line67> > > > > Seems that this class and AbstractKerberosDataFileBuilder could be in > > the same class hierarchy since they share many methods. > > > > I noticied that this implementatin of open() is a bit different that > > the implementation in AbstractKerberosDataFileBuilder. Should file be > > checked for null as it is in AbstractKerberosDataFileBuilder? `AbstractKerberosDataFileBuilder` uses `org.apache.commons.csv.CSVPrinter` `AbstractKerberosDataFileReader` uses `org.apache.commons.csv.CSVParser` - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28944/#review64968 ----------------------------------------------------------- On Dec. 12, 2014, 9:07 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28944/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2014, 9:07 p.m.) > > > Review request for Ambari, dilli dorai, John Speidel, and Robert Nettleton. > > > Bugs: AMBARI-8657 > https://issues.apache.org/jira/browse/AMBARI-8657 > > > Repository: ambari > > > Description > ------- > > Provide a facility to write (build) and read a (temporary) file used to hold > data needed for updating the configuration data of services to be configured > for Kerberos. > > The format of the file should be hidden from the user of this facility, but > will be CSV. > > Added new classes: > * > org.apache.ambari.server.serveraction.kerberos.AbstractKerberosDataFileBuilder > (mostly code moved from > org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileBuilder) > * > org.apache.ambari.server.serveraction.kerberos.AbstractKerberosDataFileReader > (mostly code moved from > org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileReader) > * org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFile > * org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileBuilder > * org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileReader > * org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileTest > > Updated existing classes to implement abstract builder and reader : > * org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileBuilder > * org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileReader > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java > 057bf18 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFile.java > f2a0f06 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileBuilder.java > b19e6f4 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileReader.java > f3d93f5 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileBuilder.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReader.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java > 788e087 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileTest.java > 71b3084 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/28944/diff/ > > > Testing > ------- > > Additional unit test: > org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileTest > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.087 sec > > Existing unit test: > org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileTest > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.102 sec > > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 26:46.040s > [INFO] Finished at: Thu Dec 11 10:39:48 EST 2014 > [INFO] Final Memory: 45M/774M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Robert Levas > >
