----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20501/#review41474 -----------------------------------------------------------
In my opinion, this patch should probably be split into 2 distinct patches. One for the base stack related changes for the new require-input functionality and another patch for the blueprint code that uses this new functionality. ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java <https://reviews.apache.org/r/20501/#comment74903> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/api/util/StackExtensionHelper.java <https://reviews.apache.org/r/20501/#comment74902> Incomplete javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74904> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74905> incomplete javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74906> incomplete javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74907> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74908> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java <https://reviews.apache.org/r/20501/#comment74909> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74910> should expand imports ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74911> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74912> missing ambariMetaInfo param in javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74916> remove throws clause. See comment in validateConfigurations() ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74917> remove throws clause. See comment in validateConfigurations() ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74918> remove throws clause. See comment in validateConfigurations() ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74913> This method is called for both create and delete operations. For delete, we should not test for required properties. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74919> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74914> This is only looking at cluster scoped configuration and is ignoring host group scoped configuration. Validation needs to be done on the complete configuration. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java <https://reviews.apache.org/r/20501/#comment74915> Shouldn't throw the generic AmbariException here as it will result in a 500 system exception back to the user instead of the expected 400 bad request. For now throw an InvalidArgumentException (not exactly correct since it is really a missing argument) which will result in a 400 back to the user. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java <https://reviews.apache.org/r/20501/#comment74920> expand imports ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java <https://reviews.apache.org/r/20501/#comment74921> missing javadoc ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java <https://reviews.apache.org/r/20501/#comment74922> It was decided that we would not use the approach of a using the lowercase service name as a default property. Instead, we will introduce a new universal_password field in the cluster create call. ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/configuration/global.xml <https://reviews.apache.org/r/20501/#comment74923> why is this line needed? ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/configuration/hive-site.xml <https://reviews.apache.org/r/20501/#comment74924> why is this a space ' ' and not null ''? ambari-server/src/main/resources/stacks/HDP/2.0.6/services/NAGIOS/configuration/global.xml <https://reviews.apache.org/r/20501/#comment74925> why is this line needed? Isn't require-input an attribute of the property tag? ambari-server/src/main/resources/stacks/HDP/2.1/services/HIVE/configuration/hive-site.xml <https://reviews.apache.org/r/20501/#comment74926> why do we have a default password value specified here? Why isn't this property marked as require-input? - John Speidel On April 23, 2014, 9:47 p.m., Sid Wagle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20501/ > ----------------------------------------------------------- > > (Updated April 23, 2014, 9:47 p.m.) > > > Review request for Ambari, Jaimin Jetly, John Speidel, Sumit Mohanty, and > Srimanth Gunturi. > > > Bugs: AMBARI-5515 > https://issues.apache.org/jira/browse/AMBARI-5515 > > > Repository: ambari > > > Description > ------- > > Hive deployment via Blueprints works when the following configurations were > added to the blueprint. > Several of the properties do exist in the stack definition, but most don't. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > 659499e > > ambari-server/src/main/java/org/apache/ambari/server/api/util/StackExtensionHelper.java > 08a545f > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > 034e877 > > ambari-server/src/main/java/org/apache/ambari/server/controller/StackConfigurationResponse.java > e5cbdf7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java > ea2fdec > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java > 18031d0 > > ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java > acc5f4a > > ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/configuration/global.xml > f3c274a > > ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/configuration/hive-site.xml > 3a6ed76 > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/configuration/global.xml > e164c64 > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/configuration/hive-site.xml > 6336a70 > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/NAGIOS/configuration/global.xml > 61a2b90 > > ambari-server/src/main/resources/stacks/HDP/2.1/services/HIVE/configuration/hive-site.xml > 781fdcb > > ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java > 3a63b78 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java > 9e12c63 > > ambari-server/src/test/resources/stacks/HDP/2.0.1/services/HIVE/configuration/hive-site.xml > 7d35558 > > Diff: https://reviews.apache.org/r/20501/diff/ > > > Testing > ------- > > mvn clean test -Djava.awt.headless=true -DfailIfNoTests=false > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Main ....................................... SUCCESS [1.966s] > [INFO] Apache Ambari Project POM ......................... SUCCESS [0.303s] > [INFO] Ambari Web ........................................ SUCCESS [9.690s] > [INFO] Ambari Views ...................................... SUCCESS [1.835s] > [INFO] Ambari Server ..................................... SUCCESS > [14:38.002s] > [INFO] Ambari Agent ...................................... SUCCESS [15.533s] > [INFO] Ambari Client ..................................... SUCCESS [0.522s] > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Sid Wagle > >
