> On April 22, 2016, 5:10 p.m., Srimanth Gunturi wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java, > > line 77 > > <https://reviews.apache.org/r/44210/diff/2/?file=1348336#file1348336line77> > > > > Trying to think if we really need to expose the 'advisor_name' and > > 'advisor_path' in the stack-service API response? The files will be at > > known locations anyways and used when available... similar to the stack's > > advisor_path/advisor_name. > > > > I am thinking this can work without adding these 2 properties to a > > stack-service. > > Tim Thorpe wrote: > The reason this works for the stack is because the stack_advisor.py in > the scripts directory attempts to load all the stack_advisor.py files. It > uses the stack name, stack version and parent versions to determine what to > load. It starts with the oldest version and keeps loading until the most > recent version. In order to do this for services, we'd need to know the some > sort of information to determine what to load from where. I could have added > something similar so that it would look into the stack versions and then > somehow look to common services if that was required. In my code I was > testing with HAWQ and PXF with their service advisor's being loaded from the > common-services directory. It seemed much easier to me at least to get the > service_advisor.py location and calculate the advisor name in the java code > when reading the stack and services (StackManager/StackModule/StackDirectory > etc...). This way I know exactly where to load the py file and exactly what > class name t o use.
Yes, I agree that it makes it easy to determine location of files. But if you look at it from pure API perspective, it is not useful for any caller except ambari-server - None of the callers have access to those paths/files except ambari-server. It is ambari-server's internals exposed outside just for its own consumption. Also, like the stack's service-advisor.py, the service's files can be dynamically located and loaded. If it is a question of efficiency, we can cache the result so that for a stack-version's service we only determine this once. My vote would be to not have these properties on the stack service's response. - Srimanth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44210/#review130123 ----------------------------------------------------------- On April 22, 2016, 6:27 p.m., Tim Thorpe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44210/ > ----------------------------------------------------------- > > (Updated April 22, 2016, 6:27 p.m.) > > > Review request for Ambari, Alexander Denissov, bhuvnesh chaudhary, Jayush > Luniya, Oleksandr Diachenko, Sumit Mohanty, Srimanth Gunturi, and Yusaku Sako. > > > Bugs: AMBARI-15226 > https://issues.apache.org/jira/browse/AMBARI-15226 > > > Repository: ambari > > > Description > ------- > > Currently the stack advisor is defined under each stack version such as > HDP/2.3. The problem with this is that it restricts the services that can be > added to the stack. If a custom service is to be added, they would need to > modify the stack advisor. If the configuration recommendation and validation > can be done at the service level then the custom service could just include > their own recommendations and validations separately. > > > Diffs > ----- > > ambari-server/src/main/assemblies/server.xml e1a4919 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > df65010 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java > 00c8696 > > ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java > ca1968e > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackServiceResourceProvider.java > 6c6fa91 > > ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java > 636de37 > > ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java > d574d60 > > ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java > b7e09a9 > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java > d27e52a > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java > db95fec > > ambari-server/src/main/resources/common-services/HAWQ/2.0.0/service_advisor.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/PXF/3.0.0/service_advisor.py > PRE-CREATION > ambari-server/src/main/resources/properties.json eac0dbd > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py > 0130483 > ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py > 3a65541 > ambari-server/src/main/resources/stacks/service_advisor.py PRE-CREATION > ambari-server/src/main/resources/stacks/stack_advisor.py 539bd25 > ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py > 6c9fd46 > > Diff: https://reviews.apache.org/r/44210/diff/ > > > Testing > ------- > > Ran all the non java unit tests. > > Total run:945 > Total errors:0 > Total failures:0 > > Manually configured HAWQ and PXF as part of the HDP 2.3 stack and made sure > their service advisors were called. > > > Thanks, > > Tim Thorpe > >
