> On April 29, 2016, 8:10 a.m., Nate Cole wrote: > > Thanks for helping me find sh!t that wasn't happening in my env.
Sure thing! I almost fell out of my chair when I saw that static injector living inside of the ECW. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46797/#review131082 ----------------------------------------------------------- On April 28, 2016, 11:09 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46797/ > ----------------------------------------------------------- > > (Updated April 28, 2016, 11:09 p.m.) > > > Review request for Ambari, Nate Cole and Sid Wagle. > > > Bugs: AMBARI-16082 > https://issues.apache.org/jira/browse/AMBARI-16082 > > > Repository: ambari > > > Description > ------- > > Recent changes to the AmbariManagementControllerTest began reusing the same > database for all test cases. This significantly dropped the test time from 5+ > minutes to about 40 seconds. > > However, there is a very bad anti-pattern in our code which caused a > different, static Injector to live inside of the ExecutionCommandWrapper. We > should be utilizing proper Guice factory patterns here to create instances of > injected objects. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapper.java > 52febc4 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperFactory.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java > 83fa6b9 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommandFactoryImpl.java > cf1e989 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java > 5a313d8 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/StageFactoryImpl.java > 9ee7c16 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 91d6b4d > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java > df033d1 > > ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java > 6cb9e6f > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java > 46c0b03 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/TaskResourceProviderTest.java > 91cfef8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java > a9c4b17 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/AutoSkipFailedSummaryActionTest.java > 5e35b301 > > ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java > 215d137 > > Diff: https://reviews.apache.org/r/46797/diff/ > > > Testing > ------- > > Tested AMCTest as well as the others changed. > > > Thanks, > > Jonathan Hurley > >
