> On Nov. 6, 2015, 9:52 a.m., Sebastian Toader wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java, > > line 92 > > <https://reviews.apache.org/r/39996/diff/1/?file=1117446#file1117446line92> > > > > Couldn't this constructor just simply call the second one to avoid code > > duplication? e.g. > > > > this(service, desiredStateEntity, injector)
Not really, one creates a fresh new in-memory only instantiation, the other reconstructs from an entity. > On Nov. 6, 2015, 9:52 a.m., Sebastian Toader wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java, > > line 353 > > <https://reviews.apache.org/r/39996/diff/1/?file=1117446#file1117446line353> > > > > You may forgot this line commented out. In general I think it's a good > > practice to reload an entity from db prior modifying it to ensure no stale > > other entities are referenced (in case the referenced entities were already > > deleted in some other place). Yes, thanks I was playing around with unit testing. > On Nov. 6, 2015, 9:52 a.m., Sebastian Toader wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java, > > line 108 > > <https://reviews.apache.org/r/39996/diff/1/?file=1117447#file1117447line108> > > > > Can this constructor call the other one with the appropriate params to > > avoid code duplication? I see your point of reducing code dup but there are more than 1 differences like persisted flag, so will leave the code asis for now. > On Nov. 6, 2015, 9:52 a.m., Sebastian Toader wrote: > > ambari-server/src/test/java/org/apache/ambari/server/testing/DBInconsistencyTests.java, > > line 72 > > <https://reviews.apache.org/r/39996/diff/1/?file=1117449#file1117449line72> > > > > I think this initialisation needs to run only once per test suite so > > would make it @BeforeClass Actually, every test starts and stop persistence service this way which avoid test from stepping on each other vs cleanup code in the @After method. - Sid ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39996/#review105387 ----------------------------------------------------------- On Nov. 5, 2015, 11:38 p.m., Sid Wagle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39996/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2015, 11:38 p.m.) > > > Review request for Ambari, Myroslav Papirkovskyy, Sumit Mohanty, and > Sebastian Toader. > > > Bugs: AMBARI-13753 > https://issues.apache.org/jira/browse/AMBARI-13753 > > > Repository: ambari > > > Description > ------- > > - Deleted hostcomponent rows re-appear > - Inconsistencies in hostcomponentstate and hostcomponentdesiredstate tables > > Preliminary patch. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java > 6150011 > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java > bbe2f62 > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java > c0804ff > > ambari-server/src/test/java/org/apache/ambari/server/testing/DBInconsistencyTests.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/39996/diff/ > > > Testing > ------- > > Unit testing in progress. > > > Thanks, > > Sid Wagle > >
