----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34671/#review85216 -----------------------------------------------------------
Ship it! Looks good Bob, thanks. One minor nit, would be good to remove the todo on line 110 now that you have introduced a stack factory. - John Speidel On May 26, 2015, 5:58 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34671/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 5:58 p.m.) > > > Review request for Ambari, John Speidel, Mahadev Konar, Robert Levas, and > Sumit Mohanty. > > > Bugs: AMBARI-11393 > https://issues.apache.org/jira/browse/AMBARI-11393 > > > Repository: ambari > > > Description > ------- > > This patch addresses AMBARI-11393. > > When a Blueprint was POST-ed with a reference to an invalid stack version > number, the Blueprints processor would throw a "500 Server Error" back to the > caller. This was due to the fact that this error case was not being properly > handled by the processor. The "500 Server Error" did not include any useful > information for debugging the failure. > > In order to address this issue, this patch implements the following: > > 1. Modifies the BlueprintFactory.createStack() method to catch an > "ObjectNotFoundException", rather than a "StackAccessException". Some code > in Ambari, including the AmbariMetaInfo class used by the BlueprintFactory, > throw "ObjectNotFoundException" or "ParentObjectNotFoundException" rather > than "StackAccessException". Since "StackAccessException" and > "ParentObjectNotFoundException" inherit from "ObjectNotFoundException", > catching "ObjectNotFoundException" allows for the most flexibility in error > handling this case. > 2. Refactors the code that creates the Stack object to use an internal > factory interface. The default implementation uses the Stack constructor as > before. This approach allows for simpler unit testing, as tests can plug in > mock factories to simulate various Stacks and Stack-related error conditions. > > 3. Adds a unit test to verify this change. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java > f02db81 > > ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java > cd465cf > > Diff: https://reviews.apache.org/r/34671/diff/ > > > Testing > ------- > > 1. Ran the ambari-server unit tests, which are all passing. > 2. Attempted a POST attempt with a Blueprint that references an invalid stack > version number ("HDP", version "99.99"), and verified that the correct error > is thrown back to the client. > 3. Ran a 3-node Blueprint cluster install with a valid Blueprint version, to > make sure this change won't break deploying valid Blueprints. > > > Thanks, > > Robert Nettleton > >
