> On Dec. 22, 2016, 3:59 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java, > > line 299 > > <https://reviews.apache.org/r/54958/diff/1/?file=1590763#file1590763line299> > > > > Would rather use new File(parent, child) syntax
suggestion has been incorporated in the 2nd revision of the patch > On Dec. 22, 2016, 3:59 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java, > > line 206 > > <https://reviews.apache.org/r/54958/diff/1/?file=1590764#file1590764line206> > > > > no need to qualify the generic for the type: new ArrayList<> is > > sufficient suggestion has been incorporated in the 2nd revision of the patch > On Dec. 22, 2016, 3:59 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java, > > lines 549-561 > > <https://reviews.apache.org/r/54958/diff/1/?file=1590764#file1590764line549> > > > > There could be a LOT of commands which seems like it would be a LOT of > > threads. Use an Executor instead. That will avoid both thread bloat and > > process handle bloat. This has been addressed in the 2nd revision of the patch - Jaimin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54958/#review159970 ----------------------------------------------------------- On Dec. 23, 2016, 12:57 a.m., Jaimin Jetly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54958/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2016, 12:57 a.m.) > > > Review request for Ambari, Jonathan Hurley, Nate Cole, Sumit Mohanty, and Sid > Wagle. > > > Bugs: AMBARI-19275 > https://issues.apache.org/jira/browse/AMBARI-19275 > > > Repository: ambari > > > Description > ------- > > Currently there is an API to download compressed config files for a host > component > > Scope of this ticket is to extend this functionality to do following: > # Download all client config files for a host via single API > # Download all client config files of a service via single API. some services > have more than one client > # Download all client config files for cluster > > Apart from extending the functionalty of API, an additional change has been > made to the behavior of the existing API that looked to be like a bug > Without this patch Downloading client config files for a service component > used to fetch first host component and create config files as present on that > host component (host desired configs overlaid on cluster desired configs) > Instead with a change in this patch when download client config is made on a > service component then config files with only cluster base configs will be > created for downoad. > > > Diffs > ----- > > ambari-server/pom.xml ceec39c > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ComponentService.java > ded2596 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java > bc9bb30 > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > d2f7934 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java > 020a454 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ComponentServiceTest.java > cb3c2a7 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/HostComponentServiceTest.java > f67d961 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java > cb88ae1 > > Diff: https://reviews.apache.org/r/54958/diff/ > > > Testing > ------- > > Tested manually on a cluster > Added validation to existing unit tests > Executed all unit tests on dev machine and checked there is no regression > Jenkins QA job for 1st revision of the patch compltedt successsfully > Jenkins QA job for 2nd revision of the patch in progress... > > > Thanks, > > Jaimin Jetly > >
