----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58415/#review171873 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/topology/validators/StackConfigTypeValidator.java Lines 41 (patched) <https://reviews.apache.org/r/58415/#comment244871> missing `<>` ambari-server/src/main/java/org/apache/ambari/server/topology/validators/StackConfigTypeValidator.java Lines 48-49 (patched) <https://reviews.apache.org/r/58415/#comment244872> missing <> ambari-server/src/main/java/org/apache/ambari/server/topology/validators/StackConfigTypeValidator.java Lines 51-56 (patched) <https://reviews.apache.org/r/58415/#comment244874> Can you please simplify this a bit? Avoid the `retainAll` call, construct the `invalidConfigTypes` set (use `Sets.difference`), and check if it has any elements. ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterDeployWithStartOnlyTest.java Line 61 (original), 57 (patched) <https://reviews.apache.org/r/58415/#comment244869> unused import - Attila Doroszlai On April 13, 2017, 4:13 p.m., Laszlo Puskas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58415/ > ----------------------------------------------------------- > > (Updated April 13, 2017, 4:13 p.m.) > > > Review request for Ambari, Attila Doroszlai, Robert Levas, Sandor Magyari, > and Sebastian Toader. > > > Bugs: AMBARI-20755 > https://issues.apache.org/jira/browse/AMBARI-20755 > > > Repository: ambari > > > Description > ------- > > Problem: > Configuration types posted in the blueprint or in the cluster creation > template need to be validated before the cluster provisioning is started and > cluster resources are persisted in the database. > This validation should be done when the topology configuration comes together > so that it's not modified and enhanced further. > Currently the topology configuration is modified after the validation thus on > blueprint deployments the cluster provisioning may fail despite of the right > configuration. > > Notes: > - due to the nature of the topology validation, the validation logic and > management of the validators have been extracted to a specialized service > (before validators were attached to requests and the validation happened in > the constructor) > - some unit test cases became unecessary due to the above change, i removed > them (and added other unit tests) > - there are some formatting related hunks in the diff (i added them by > mistake) > > The validators have been modified (as per internal discussions and test > results) > - configuration types are considered invalid if they are not in the stack ( > regardless the services being deployed) > - the hive database related validation checks for the mysql server in the > list of components, not in the list of services > > The patch is a draft, i am still massaging it (adding comments, cleaning > etc...) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java > f24c138 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java > 1a14b01 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java > b5d2f9d > > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java > e37c68d > > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java > 37fb7d4 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 2ac9950 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > 392a53e > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyRequest.java > cbc6642 > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/ChainedTopologyValidator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/HiveServiceValidator.java > 1351739 > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredPasswordValidator.java > 591a124 > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/StackConfigTypeValidator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/TopologyValidatorFactory.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/validators/TopologyValidatorService.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > dba4043 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequestTest.java > 73a80f6 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ScaleClusterRequestTest.java > 48d1351 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterDeployWithStartOnlyTest.java > a691cbc > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartOnComponentLevelTest.java > 98ba592 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java > fc7ac27 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterTopologyImplTest.java > 3ea17b4 > > ambari-server/src/test/java/org/apache/ambari/server/topology/RequiredPasswordValidatorTest.java > 4c88247 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > 2d5978b > > ambari-server/src/test/java/org/apache/ambari/server/topology/validators/HiveServiceValidatorTest.java > 745b01b > > ambari-server/src/test/java/org/apache/ambari/server/topology/validators/StackConfigTypeValidatorTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58415/diff/1/ > > > Testing > ------- > > Unit tests ran against trunk. (some unrelatet pyhon tests failed though) > Manually tested on the local env. (Still need to do some testing) > > > Thanks, > > Laszlo Puskas > >