----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58990/#review173901 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredConfigPropertiesValidator.java Lines 38 (patched) <https://reviews.apache.org/r/58990/#comment246980> Aren' there some password properities that are required when installing a service? If so, shouldn't the existance of the property be tested? Being that the fill set of configs is used when validating, I would expect that a required password property would have been set in the Cluster Creation Template. ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredConfigPropertiesValidator.java Lines 89 (patched) <https://reviews.apache.org/r/58990/#comment246983> Isn't it possible for host groups to have different sets of configuations. This method appears to combine all hosts group-specific configs into one _common_ configuration map. ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredConfigPropertiesValidator.java Lines 96 (patched) <https://reviews.apache.org/r/58990/#comment246984> This will overwrite the _global_ configration map entries with the host group-specific configration map entries. So if the _global_ configiration map contains: ``` foo-type: {property1:a, property2:b} ``` And a host group configuration map contains: ``` foo-type: {property1:c, property3:d} ``` The end result will contain: ``` foo-type: {property1:c, property3:d} ``` Not ``` foo-type: {property1:c, property2:b, property3:d} ``` Which would still be incorrect for the scenarip where multipe host groups specifiy a property map for `foo-type`. - Robert Levas On May 4, 2017, 9:38 a.m., Laszlo Puskas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58990/ > ----------------------------------------------------------- > > (Updated May 4, 2017, 9:38 a.m.) > > > Review request for Ambari, Attila Doroszlai, Robert Levas, Robert Nettleton, > and Sandor Magyari. > > > Bugs: AMBARI-20872 > https://issues.apache.org/jira/browse/AMBARI-20872 > > > Repository: ambari > > > Description > ------- > > Validating required properties for services listed in the blueprint can't be > accurate as further configurtion can be added in the cluster creation > template. > (Where the configuration is added varies by use cases and usage) > > This patch contains the change that defers this validation till all the > configuration is together, namely till the cluster creation template is > posted. > The validation logic is the same as it was before (provided config entries > are checked against those marked as required in the stack) the way the > properties are collected has slightly changed: > > - before the properties were retrieved by traversing hostgroups > - in the current implementation required properties are retrieved by > traversing the services listed in the blueprint > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintValidatorImpl.java > 45a8c5c > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyValidator.java > 146b424 > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredConfigPropertiesValidator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/TopologyValidatorFactory.java > 0e77301 > > ambari-server/src/test/java/org/apache/ambari/server/topology/validators/RequiredConfigPropertiesValidatorTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58990/diff/1/ > > > Testing > ------- > > Tested successfully on local environment. > Added unit tests. > > Unit tests running ... > > > Thanks, > > Laszlo Puskas > >
