> On March 23, 2015, 4:25 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FinalizeUpgradeAction.java, > > line 210 > > <https://reviews.apache.org/r/32413/diff/1/?file=903388#file903388line210> > > > > Can you explain this null check? Under what conditions would the OS for > > the repo versions be null? If they could be null, it seems like finalize > > wouldn't update the repo base URL. > > Nate Cole wrote: > I guess it couldn't - I only looked at the member variable, which has no > default. Looks like the getter is making appropriate choices. Will remove.
Yeah, it's always good to be cautious around NPEs that are returned. In this case, the fix wouldn't run and it seemed like something that we should be sure about. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32413/#review77456 ----------------------------------------------------------- On March 23, 2015, 4:20 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32413/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 4:20 p.m.) > > > Review request for Ambari, Alejandro Fernandez and Jonathan Hurley. > > > Bugs: AMBARI-10189 > https://issues.apache.org/jira/browse/AMBARI-10189 > > > Repository: ambari > > > Description > ------- > > After finalizing, when adding a new host, the stack-defined repo base_url is > used. This is incorrect, as the cluster current should be used instead. > > The fix for this issue could be done by either: > 1. Fix finalize to set the stack version. > 2. Fix the repo info code to pull from cluster current. > > #1 is quicker and least impact/easier to test. The proper fix is #2 but > requires more careful thought and more time than is allotted. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FinalizeUpgradeAction.java > 01bd9c7 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeActionTest.java > c107d89 > > Diff: https://reviews.apache.org/r/32413/diff/ > > > Testing > ------- > > Manual + new unit test. > > Automated results pending > > > Thanks, > > Nate Cole > >
