> On June 12, 2015, 2:52 p.m., Jonathan Hurley wrote: > > Why are you defaulting to "2.2.4.2" in configuration? That's an arbitrary > > value. Instead of building this into Configuration, you should be updating > > the pre-req checks to have knowledge of the stacks that they work with.
Hello Jonathan, I rewrote the logic to 1. remove references to any configurations in the ambari-server's properties file 2. updated the AbstractCheckDiscriptor to use information passed in from the PrecheckRequest. Could you please help review it? > On June 12, 2015, 2:52 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java, > > line 74 > > <https://reviews.apache.org/r/35352/diff/1/?file=982689#file982689line74> > > > > This should be injected. I rewrote the logic no need to inject any properties values into the AbstractCheckDescriptor. > On June 12, 2015, 2:52 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java, > > lines 130-131 > > <https://reviews.apache.org/r/35352/diff/1/?file=982689#file982689line130> > > > > Inject instead I rewrote the logic no need to inject any properties values into the AbstractCheckDescriptor. > On June 12, 2015, 2:52 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java, > > line 135 > > <https://reviews.apache.org/r/35352/diff/1/?file=982689#file982689line135> > > > > It seems very wrong to be pulling this from Configuration; why not just > > get this from the target version that's passed in? I rewrote the logic to use info passed in from the precheckrequest object. - Di ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35352/#review87716 ----------------------------------------------------------- On June 12, 2015, 2:45 p.m., Di Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35352/ > ----------------------------------------------------------- > > (Updated June 12, 2015, 2:45 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate > Cole. > > > Bugs: AMBARI-11810 > https://issues.apache.org/jira/browse/AMBARI-11810 > > > Repository: ambari > > > Description > ------- > > All the rolling upgrade pre-checks may still need to run against a non-HDP > stack. Use this JIRA to make the check runnable based on properties > (rollingupgrade.stack, rollingupgrade.version) set in the > /etc/ambari-server/conf/ambari.properties. Still default to HDP and 2.2.4.2 > if the properties do not exist. > > org.apache.ambari.server.checks.AbstractCheckDescriptor compares stack > against hardcoded StackInfo "HDP-2.2". the stack name and version info can be > externalized into the /etc/ambari-server/conf/ambar.properties in order to > support non-HDP stacks. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java > 8cabf29 > > ambari-server/src/test/java/org/apache/ambari/server/checks/AbstractCheckDescriptorTest.java > 4834cee > > Diff: https://reviews.apache.org/r/35352/diff/ > > > Testing > ------- > > Added new unit tests > > Manually tested the fix by kicking off the rolling upgrade > 1. With no properties in /etc/ambari-server/conf/ambari.properties, Java code > used the default HDP-2.2 StackId information and performed the upgrade > pre-checks. > 2. With wrong property values in ambari.properties, Java code threw err about > the StackId did not match > 3. With correct property values in ambari.properties, Java code used the > stack id and performed the upgrade pre-checks. > > > Thanks, > > Di Li > >
