----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30174/#review69229 -----------------------------------------------------------
ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox.py <https://reviews.apache.org/r/30174/#comment113858> Creation of these config files between Windows and Linux is quite similar and there should be a better way to avoid forking :( For now lets leave it as is to avoid impacting 2.0 but we should look at it in 2.1 ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox.py <https://reviews.apache.org/r/30174/#comment113852> Use os.path.join instead of format. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox.py <https://reviews.apache.org/r/30174/#comment113853> Use os.path.join instead of format. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox.py <https://reviews.apache.org/r/30174/#comment113854> Use os.path.join instead of format. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py <https://reviews.apache.org/r/30174/#comment113864> Instead of forking each function, might fork at class level using OsFamilyImpl instead of OsFamilyFuncImpl. Have a base class for common functionality. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py <https://reviews.apache.org/r/30174/#comment113855> Do we need to add get_stack_to_component() for Windows? ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py <https://reviews.apache.org/r/30174/#comment113860> Again here the only thing we do differently is that we check if the service has been installed. Long term this can be further refactored. Leave as is for now. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py <https://reviews.apache.org/r/30174/#comment113856> Use os.path.join ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/ldap.py <https://reviews.apache.org/r/30174/#comment113863> what if params.knox_group=None and mode=None for Windows? Will it not work? That way we dont have to fork the function for Linux and Windows. I dont like we having to fork just for that. Also we should use os.path.join everywhere. ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/params.py <https://reviews.apache.org/r/30174/#comment113861> Nice :) ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/service_check.py <https://reviews.apache.org/r/30174/#comment113865> Fork at class level instead of function level. ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/service_check.py <https://reviews.apache.org/r/30174/#comment113866> Fork at class level instead of service level. ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/slider.py <https://reviews.apache.org/r/30174/#comment113867> Again this is very repetitive. We should see if we can do away with forking. ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/slider.py <https://reviews.apache.org/r/30174/#comment113868> should be slider-env.cmd and not slider-env.sh ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/slider.py <https://reviews.apache.org/r/30174/#comment113869> Use os.path.join ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/gateway-log4j.xml <https://reviews.apache.org/r/30174/#comment113874> Is there any difference in the log4j between the common services version? We shouldnt require this if gateway-log4j.xml is the same as in common-services. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/gateway-site.xml <https://reviews.apache.org/r/30174/#comment113875> Do not duplicate all properties from the common services. Inhertence works at property level and not config file level. So if property gateway.port is the same in common-services/KNOX/.../.../gateway-site.xml you dont need to add it here. Rule of thumb: Only properties that are different from the common services version should be overriden in the stack definition. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/gateway-site.xml <https://reviews.apache.org/r/30174/#comment113877> This path need to be updated for Windows? ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/knox-env.xml <https://reviews.apache.org/r/30174/#comment113878> knox_user should be marked as deleted as in Windows we dont have knox_user. See other service definitions for reference. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/knox-env.xml <https://reviews.apache.org/r/30174/#comment113879> Should be marked as deleted. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/ldap-log4j.xml <https://reviews.apache.org/r/30174/#comment113880> Again remove this if duplicate of common-services version. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/ranger-knox-plugin-properties.xml <https://reviews.apache.org/r/30174/#comment113883> Remove duplicate properties ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/ranger-knox-plugin-properties.xml <https://reviews.apache.org/r/30174/#comment113882> Update KNOX_HOME path ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/metainfo.xml <https://reviews.apache.org/r/30174/#comment113887> Might have to override slider-env config file See HDPWIN/2.1/services/HDFS/metainfo.xml as example. <metainfo> <schemaVersion>2.0</schemaVersion> <services> <service> <name>HDFS</name> <extends>common-services/HDFS/2.1.0.2.0</extends> <version>2.4.0.2.1.1.0</version> <components> <component> <name>HDFS_CLIENT</name> <configFiles> <configFile> <type>env</type> <fileName>hadoop-env.cmd</fileName> <dictionaryName>hadoop-env</dictionaryName> </configFile> </configFiles> </component> </components> </service> </services> </metainfo> ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/SLIDER/configurations/slider-env.xml <https://reviews.apache.org/r/30174/#comment113885> These contents are wrong. For Windows shell script wont work. You should simply set the value as empty string. If we need we can update it to .cmd script. ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/SLIDER/metainfo.xml <https://reviews.apache.org/r/30174/#comment113886> Might have to override slider-env config file See HDPWIN/2.1/services/HDFS/metainfo.xml as example. <metainfo> <schemaVersion>2.0</schemaVersion> <services> <service> <name>HDFS</name> <extends>common-services/HDFS/2.1.0.2.0</extends> <version>2.4.0.2.1.1.0</version> <components> <component> <name>HDFS_CLIENT</name> <configFiles> <configFile> <type>env</type> <fileName>hadoop-env.cmd</fileName> <dictionaryName>hadoop-env</dictionaryName> </configFile> </configFiles> </component> </components> </service> </services> </metainfo> - Jayush Luniya On Jan. 22, 2015, 5:06 p.m., Artem Baranchuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30174/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2015, 5:06 p.m.) > > > Review request for Ambari, Eugene Chekanskiy, Florian Barca, and Jayush > Luniya. > > > Bugs: AMBARI-9270 > https://issues.apache.org/jira/browse/AMBARI-9270 > > > Repository: ambari > > > Description > ------- > > Add SLIDER, KNOX service to Windows stack 2.2 > > > Diffs > ----- > > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox.py > 7d7d20c > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/knox_gateway.py > 8593c5a > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/ldap.py > 2ff8297 > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/params.py > 28fabe5 > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/params_linux.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/params_windows.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/service_check.py > e05262f > > ambari-server/src/main/resources/common-services/KNOX/0.5.0.2.2/package/scripts/service_mapping.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/params.py > fbb1973 > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/params_linux.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/params_windows.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/service_check.py > af085b8 > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/slider.py > 48c534e > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/package/scripts/slider_client.py > 2c99c54 > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/gateway-log4j.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/gateway-site.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/knox-env.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/ldap-log4j.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/ranger-knox-plugin-properties.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/topology.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/configuration/users-ldif.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/KNOX/metainfo.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/SLIDER/configurations/slider-client.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/SLIDER/configurations/slider-env.xml > PRE-CREATION > > ambari-server/src/main/resources/stacks/HDPWIN/2.2/services/SLIDER/metainfo.xml > PRE-CREATION > > Diff: https://reviews.apache.org/r/30174/diff/ > > > Testing > ------- > > Unix tests passed > Win cluster deployed success > > > Thanks, > > Artem Baranchuk > >
