You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Robert Levas <rl...@hortonworks.com> on 2014/12/11 17:01:45 UTC
Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/
-----------------------------------------------------------
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
* org.apache.ambari.server.serveraction.kerberos.AbstractKerberosDataFileReader
* 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/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/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
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by Robert Levas <rl...@hortonworks.com>.
> 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
>
>
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/#review64968
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java
<https://reviews.apache.org/r/28944/#comment107800>
Just checking that this comment makes sense to you as it doesn't to me.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java
<https://reviews.apache.org/r/28944/#comment107801>
just checking that this class doesn' need to be thread safe
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java
<https://reviews.apache.org/r/28944/#comment107811>
could inline writeHeader as it is only used here
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107802>
implement -> implements
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107816>
why 2 empty lines between methods?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107817>
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?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107818>
NPE if already closed
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107819>
an Iterator of what?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107821>
NPE if closed
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java
<https://reviews.apache.org/r/28944/#comment107826>
This is a design pattern that has fallen out of favor in the java community. By placing these constants in an interface and then inheriting them, they become part of the api when they are really an implementation detail. Additionally, the hierarchy of the implementing class is now odd in that it is implementing this class with no symantic meaning.
Would be better to use static imports for this.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java
<https://reviews.apache.org/r/28944/#comment107823>
javadocs for constants
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java
<https://reviews.apache.org/r/28944/#comment107822>
why an empty line here?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileBuilder.java
<https://reviews.apache.org/r/28944/#comment107827>
See comment in KerberosDataFile about using static imports instead of inheritence.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReader.java
<https://reviews.apache.org/r/28944/#comment107828>
See comment in KerberosDataFile about using static imports instead of inheritence.
Also, it is odd to extend a class and provide no new or modified behavior.
- John Speidel
On Dec. 11, 2014, 4:04 p.m., Robert Levas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28944/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2014, 4:04 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/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/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
>
>
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/#review65019
-----------------------------------------------------------
Merged to trunk.
I added the static imports mentioned in the comments.
- John Speidel
On Dec. 13, 2014, 2:16 a.m., Robert Levas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28944/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2014, 2:16 a.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
>
>
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by Robert Levas <rl...@hortonworks.com>.
> On Dec. 12, 2014, 10:03 p.m., John Speidel wrote:
> > Let me know if you want to upload another patch for these comments or if you are ok with me merging this patch.
It's a litte late, but I am fine with it.. ;)
> On Dec. 12, 2014, 10:03 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java, line 117
> > <https://reviews.apache.org/r/28944/diff/3/?file=791155#file791155line117>
> >
> > Why not use a static import
> > import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.KEYTAB_FILE_PATH ?
static imports don't give me that warm and fuzzy feeling I get when I see out-right were stuff comes from. But if this is they way we like it, I will adapt.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/#review65015
-----------------------------------------------------------
On Dec. 12, 2014, 9:16 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:16 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
>
>
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/#review65015
-----------------------------------------------------------
Ship it!
Let me know if you want to upload another patch for these comments or if you are ok with me merging this patch.
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java
<https://reviews.apache.org/r/28944/#comment107934>
Why not use a static import
import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.KEYTAB_FILE_PATH ?
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java
<https://reviews.apache.org/r/28944/#comment107935>
same comment a precdeing line
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
<https://reviews.apache.org/r/28944/#comment107936>
static import?
- John Speidel
On Dec. 13, 2014, 2:16 a.m., Robert Levas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28944/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2014, 2:16 a.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
>
>
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/
-----------------------------------------------------------
(Updated Dec. 12, 2014, 9:16 p.m.)
Review request for Ambari, dilli dorai, John Speidel, and Robert Nettleton.
Changes
-------
Added left out fixes.
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 (updated)
-----
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
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
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.
Changes
-------
Updates to address comments
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 (updated)
-----
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
Re: Review Request 28944: Add Kerberos Configuration Metadata File
Builder and Reader
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28944/
-----------------------------------------------------------
(Updated Dec. 11, 2014, 11:04 a.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 (updated)
-------
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/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/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