> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-common/src/main/python/resource_management/libraries/functions/stack_features.py,
> >  line 32
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309634#file1309634line32>
> >
> >     Use default function instead? 
> >     
> >       from resource_management.libraries.functions.default import default
> >       stack_features_config = 
> > default("/configurations/cluster-env/stack_tools", None)
> >       data = _DEFAULT_STACK_FEATURES
> >       if stack_features_config:
> >         data = json.loads(stack_features_config)

It uses now default method and set the DEFAULT_STACK_FEATURES as value by 
default


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/params_linux.py,
> >  line 35
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309635#file1309635line35>
> >
> >     Add:-
> >     
> >     stack_root = Script.get_stack_root()

stack_root is defined in status_params.py, since is needed there, and imported 
in params_linux.py:
stack_root = status_params.stack_root


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/params_linux.py,
> >  line 55
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309635#file1309635line55>
> >
> >     Replace "/usr/hdp" with "{stack_root}"

"/usr/hdp" paths have been replaced using stack_root  parameter in 
params_linux, status_params, zookeeper_service , zookeeper  python scripts.


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/params_linux.py,
> >  line 58
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309635#file1309635line58>
> >
> >     Leave config_dir logic as is as noted in my comment on status_params.py

I keept it as it was before. config_dir definition is in status_params and 
imported in params_linux
config_dir = status_params.config_dir


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/status_params.py,
> >  line 50
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309636#file1309636line50>
> >
> >     We need to define config_dir in status_params as it will cause issue 
> > during zookeeper_server::security_status checks as only status_params is 
> > imported in security_status()
> >     
> >       def security_status(self, env):
> >         import status_params
> >         env.set_params(status_params)
> >         if status_params.security_enabled:   
> >         ...
> >           security_params = 
> > get_params_from_filesystem(status_params.config_dir,
> >                                                        
> > {'zookeeper_jaas.conf': FILE_TYPE_JAAS_CONF})
> 
> Juanjo  Marron wrote:
>     Yes, thats true. My idea was to keep the version comparison at 
> params_linux.py level
>     
>     To  maintain the logic I would need something like this in status_params:
>     
>     config_dir = "/etc/zookeeper/conf"
>     if stack_version_formatted and 
> check_stack_feature(StackFeature.ROLLING_UPGRADE, stack_version_formatted):
>          config_dir = 
> format("{stack_root}/current/{component_directory}/conf")
>     
>     stack_version_formatted , check_stack_feature , constants and stack_root 
> would need to be imported here at status_params.py level and they are 
> currently also imported at params_linux.py level. 
>     
>     Due to import status_params in param_linux, it would be enought to 
> declare them in status_params: 
>     Should I remove them from params_linux.py? 
>     Should I keep them duplicated?
>     
>     I couldnt find a good example in other services
> 
> Jayush Luniya wrote:
>     Either works as far as it works. If you can avoid duplication and ensure 
> everything works well and good.

The needed parameters are defined  in status_params and imported in 
params_linux. For example:
stack_version_formatted = status_params.stack_version_formatted
stack_root = status_params.stack_root


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/zookeeper_server.py,
> >  line 132
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309639#file1309639line132>
> >
> >     We are not importing params, only status_params (See line#115). Lets 
> > keep this as is.

I put config_dir back to status_params


> On March 22, 2016, 1:38 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.0.6/properties/stack_features.json,
> >  line 3
> > <https://reviews.apache.org/r/45141/diff/1/?file=1309642#file1309642line3>
> >
> >     I believe max_version for snappy should be 2.2.0.0?

It is corrected


- Juanjo


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


On March 22, 2016, 11:13 p.m., Juanjo  Marron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45141/
> -----------------------------------------------------------
> 
> (Updated March 22, 2016, 11:13 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jayush Luniya.
> 
> 
> Bugs: AMBARI-14435
>     https://issues.apache.org/jira/browse/AMBARI-14435
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Apply the  stack featurization prototype detailed on AMBARI-13364 to 
> ZOOKEEPER service plus the comments and reviews received on:
> https://reviews.apache.org/r/44933/
> 
> This patch removes the hardcoded stack versions in ZK common-services code 
> (centralized version and parameterized stack configurations will be faced in 
> other JIRAS)
> 
> stack_features is now a property in .json format in 
> HDP/2.0.6/properties/stack_features.json
> With the structure:
> {
> "stack_features": [
> { "name": "feature1", "description" : "Feature1 support", "min_version" : 
> "Y.Y.Y.Y" , "max_version" : "X.X.X.X"}
> , 
> ...
> ]
> }
> where min_version/max_version are optional constraints.
> 
> The upgrade constants, such has ROLLING_UPGRADE = "rolling_upgrade" has been 
> added to in a new StackFeature in 
> resource_management/libraries/functions/constants.py
> 
> It is added as part of the HDP stack configurations on 
> /HDP/2.0.6/configuration/cluster-env.xml, introducing a new stack_features 
> property
> 
> New resource_management/libraries/functions/stack_fetaures.py has been 
> introduced to parse the json file and called from service code to check if 
> the stack supports the required feature.
> 
> The stack version used in params_linux.py comparison is  
> stack_version_formatted = hostLevelParams\stack_version
> 
> 
> Diffs
> -----
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/constants.py
>  0adaef9 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_features.py
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/params_linux.py
>  9ff9125 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/status_params.py
>  9ae8440 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/zookeeper.py
>  5712ce4 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/zookeeper_client.py
>  25ace24 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/zookeeper_server.py
>  d5b6898 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5.2.0/package/scripts/zookeeper_service.py
>  6fe0772 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/configuration/cluster-env.xml
>  5f77f48 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/properties/stack_features.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45141/diff/
> 
> 
> Testing
> -------
> 
> Zookeeper fresh installation on top of latest trunk
> 
> 
> Thanks,
> 
> Juanjo  Marron
> 
>

Reply via email to