-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44835/#review123657
-----------------------------------------------------------



By the logic in this review, does "current" belong with STACK_ROOT_PATTERN?  
The amount of changes here are significant - is there a smaller piece we can do 
first to get the pattern down first?  Also, how do we account for the case 
where there are two possible roots:

/usr/hdp/current/hadoop-client which is the symlink to 
/usr/hdp/<version>/hadoop.  There are times where we need the former, and other 
cases for the latter.  How is that done?

And, what if a stack has no selector or tools?  Are we just going to error out?

It's fine to generalize how stacks represent how to do these things, but you're 
just shuffling around how those are getting determined in this review.

Did you consider refactoring instead the common-services stuff to consult the 
stack for python-sourced modules?  That would be the cleaner approach over 
trying to make up command paths on the fly based on what's on the stack and 
making things nearly impossible to debug.  We can then keep HDP-specific in the 
HDP stack directories.

- Nate Cole


On March 15, 2016, 2:32 a.m., Jayush Luniya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44835/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 2:32 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Juanjo  
> Marron, Nate Cole, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15420
>     https://issues.apache.org/jira/browse/AMBARI-15420
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Refactor resource_management library, to generalize the library and remove 
> HDP-specific hardcodings
> 
> Specifically,
> 1. Make stack-selector, conf-selector tools stack-driven instead of 
> hardcoding hdp-select and conf-select
> 2. Make stack-root stack-driven instead of hardcoding /usr/hdp
> 3. Make copy_tarball mappings stack-driven 
> 4. Make PACKAGE_DIRS mapping in conf_select use stack-root instead of 
> hardcoding the "/usr/hdp"
> 
> In addition, also added a feature in the stack processing engine to support 
> properties values to be defined a external property file (See 
> tarball_map.json, stack_tools.json in patch)
> 
> Three config properties are added
> 1. cluster-env/stack_root
> 2. cluster-env/stack_tools
> 3. cluster-env/tarball_map
> Corresponding helper functions get_stack_root(), get_stack_tool(), 
> get_tarball_map() are added in script.py, which will set the defaults if 
> these config properties are not defined (ambari-server upgrade scenario needs 
> to be addressed to add these config properties on upgrade). These helper 
> functions are used to remove hardcodings in resource_management library.
> 
> 
> Remaining HDP-specific logic in resource_management library
> 
> 1. conf_select::_valid()
> 2. conf_select::get_hadoop_conf_dir()
> 3. list_ambari_managed_repos::repository_names
> 4. version_select_util::get_component_version()
> 5. script::get_stack_version()
> 6. script::should_expose_component_version()
> 7. get_lzo_packages::get_lzo_packages()
> 
> Refactoring this HDP-specific logic would require "Stack Featurization" 
> (AMBARI-13364) to be in place.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/HostCheckReportFileHandler.py 
> ee7db0a 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/conf_select.py
>  59c717b 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/copy_tarball.py
>  647b8b6 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/default.py
>  23383dc 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/dynamic_variable_interpretation.py
>  a20b03c 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
>  f2e6567 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/install_windows_msi.py
>  f1cd9cb 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/repo_version_history.py
>  d585dea 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/setup_ranger_plugin.py
>  4d9d8a4 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/setup_ranger_plugin_xml.py
>  2ccc0c6 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_select.py
>  c94d956 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/version_select_util.py
>  95c5cba 
>   
> ambari-common/src/main/python/resource_management/libraries/script/script.py 
> a8098a0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
>  4be4049 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ConfigurationDirectory.java
>  7f21aaa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> 0c7faea 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java
>  c739211 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 7d934bb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> e7c9c27 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/ValueAttributesInfo.java
>  3f7f756 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/JsonUtils.java 
> PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ACCUMULO/1.6.1.2.2.0/package/scripts/accumulo_script.py
>  12ca388 
>   
> ambari-server/src/main/resources/common-services/ACCUMULO/1.6.1.2.2.0/package/scripts/status_params.py
>  45dbb24 
>   
> ambari-server/src/main/resources/common-services/ATLAS/0.1.0.2.3/package/scripts/atlas_client.py
>  d000846 
>   
> ambari-server/src/main/resources/common-services/FALCON/0.5.0.2.1/package/scripts/status_params.py
>  2c06c40 
>   
> ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/status_params.py
>  535c821 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hcat_client.py
>  85e7012 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/hive_server_upgrade.py
>  c3d15e5 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/params_linux.py
>  c5e61e6 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/status_params.py
>  d0924b9 
>   
> ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py
>  30b9a41 
>   
> ambari-server/src/main/resources/common-services/OOZIE/4.0.0.2.0/package/scripts/oozie.py
>  81a227e 
>   
> ambari-server/src/main/resources/common-services/OOZIE/4.0.0.2.0/package/scripts/oozie_server.py
>  030fb2d 
>   
> ambari-server/src/main/resources/common-services/OOZIE/4.0.0.2.0/package/scripts/oozie_server_upgrade.py
>  27e2766 
>   
> ambari-server/src/main/resources/common-services/OOZIE/4.0.0.2.0/package/scripts/status_params.py
>  954bb80 
>   
> ambari-server/src/main/resources/common-services/RANGER/0.4.0/package/scripts/params.py
>  e5b54cd 
>   
> ambari-server/src/main/resources/common-services/SPARK/1.2.0.2.2/package/scripts/params.py
>  906b198 
>   
> ambari-server/src/main/resources/common-services/SQOOP/1.4.4.2.0/package/scripts/params_linux.py
>  ec37243 
>   
> ambari-server/src/main/resources/common-services/STORM/0.9.1.2.1/package/scripts/status_params.py
>  984a4ba 
>   
> ambari-server/src/main/resources/common-services/TEZ/0.4.0.2.1/package/scripts/params_linux.py
>  eb80ad6 
>   
> ambari-server/src/main/resources/common-services/TEZ/0.4.0.2.1/package/scripts/params_windows.py
>  dd732f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  8b2aec5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/yarn.py
>  e05ed60 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/status_params.py
>  d18e4d7 
>   ambari-server/src/main/resources/custom_actions/scripts/install_packages.py 
> 08bdcc3 
>   ambari-server/src/main/resources/custom_actions/scripts/remove_bits.py 
> e69a2e4 
>   ambari-server/src/main/resources/custom_actions/scripts/ru_set_all.py 
> b8bf176 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/configuration/cluster-env.xml
>  3fb82e9 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/after-INSTALL/scripts/shared_initialization.py
>  96dc104 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/before-INSTALL/scripts/shared_initialization.py
>  07faae4 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/properties/stack_tools.json 
> PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/properties/tarball_map.json 
> PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/ServiceModuleTest.java
>  92c1200 
>   
> ambari-server/src/test/java/org/apache/ambari/server/utils/TestJsonUtils.java 
> PRE-CREATION 
>   ambari-server/src/test/python/custom_actions/test_ru_set_all.py 3090f6b 
>   ambari-server/src/test/python/host_scripts/TestAlertDiskSpace.py 2608050 
>   ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b5b43d6 
>   ambari-server/src/test/python/stacks/2.0.6/YARN/test_mapreduce2_client.py 
> efe6038 
>   ambari-server/src/test/python/stacks/2.0.6/YARN/test_yarn_client.py 4601092 
>   
> ambari-server/src/test/python/stacks/2.0.6/hooks/after-INSTALL/test_after_install.py
>  daee726 
>   ambari-server/src/test/python/stacks/2.1/TEZ/test_tez_client.py ab08776 
> 
> Diff: https://reviews.apache.org/r/44835/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jayush Luniya
> 
>

Reply via email to