----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55424/#review161228 -----------------------------------------------------------
Fix it, then Ship it! ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 123) <https://reviews.apache.org/r/55424/#comment232471> Kind of a double-negative - can you make this if(isUpgrade()) instead? ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 213) <https://reviews.apache.org/r/55424/#comment232472> Documentation. ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 219) <https://reviews.apache.org/r/55424/#comment232473> Null Check and throw exception? ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 230) <https://reviews.apache.org/r/55424/#comment232474> Documentation. ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (lines 245 - 247) <https://reviews.apache.org/r/55424/#comment232475> I'm worried about this part. If something goes wrong on the downgrade in the next method and the user re-tries the command, it won't be able to find the backup, right? I know you've already restored it - but it's good to keep the backup just in case you need it. - Jonathan Hurley On Jan. 11, 2017, 8:32 a.m., Eugene Chekanskiy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55424/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2017, 8:32 a.m.) > > > Review request for Ambari, Dmytro Grinenko, Jonathan Hurley, and Robert Levas. > > > Bugs: AMBARI-19464 > https://issues.apache.org/jira/browse/AMBARI-19464 > > > Repository: ambari > > > Description > ------- > > AMBARI-18335 addresses an issue where configuration properties in the > Kerberos descriptor don't change during a stack upgrade. It accounts for both > upgrade/downgrade scenarios and situations where properties are added/removed. > However, it does not take into account the potential for a property to be > customized by the user (not a new property, but a property which the user > changes the value of). In this case, the downgrade would be destructive, > overriding the user's values with those from the downgrade stack. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ArtifactEntity.java > 8972e6d > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java > f1eab38 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/55424/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Eugene Chekanskiy > >