ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sid Wagle" <swa...@hortonworks.com>
Subject Re: Review Request 20346: Stack definition does not provide global properties with empty values
Date Wed, 16 Apr 2014 18:27:44 GMT


> On April 15, 2014, 6:30 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/util/StackExtensionHelper.java,
line 597
> > <https://reviews.apache.org/r/20346/diff/1/?file=557721#file557721line597>
> >
> >     Why would the property name be null or empty?  Was there an issue which prompted
adding this check?
> 
> Sid Wagle wrote:
>     Correct, there was empty property and it resulted in an UI issue.
> 
> John Speidel wrote:
>     Wouldn't it be better to remove the offending property from the stack?  How is a
property with no name useful?

Done that as well, this is added as blanket check.


> On April 15, 2014, 6:30 p.m., John Speidel wrote:
> > ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HBASE/configuration/global.xml,
lines 25-34
> > <https://reviews.apache.org/r/20346/diff/1/?file=557722#file557722line25>
> >
> >     Why are we deleting properties which previously existed but had no values? 
Are these properties no longer used?  This comment applies to all properties which were removed
across all of the diffs.  
> >     
> >     There are some deleted properties which seem that they are still used and would
have valid default values such as hbase.master.info.port ...
> >     
> >
> 
> Sid Wagle wrote:
>     The deleted properties are of the form "_host(s)". These are used to convey cluster
topology info to the agent. The server sends these without looking at the stack properties
so these were pretty much placeholder with current code.
>     
>     Another point is, if someone wants to add a Service we cannot rely on topology related
properties to be present in the global config of a new service definition. 
>     
>     I have opened a new Jira for standardizing on the way topology info is sent, basically
something like, componentName_category_host(s).
> 
> John Speidel wrote:
>     ok.  But why would be delete properties such as hbase-site/hbase.master.info.port?
> 
> Sid Wagle wrote:
>     The properties deleted like "hbase.master.info.port" were not set by the UI while
saving configs. This is an optional override that the user can provide if needed. I had the
option of either providing a value corresponding to what is checked into HBase trunk or remove
the property which would represent what happens with a default cluster install. Note: If I
do not remove the property and leave it as empty the UI code needs to handle the property,
by making it not required, or setting a value. None of these property have empty as valid
default.
> 
> John Speidel wrote:
>     I think that it is important to include these properties in the stack with default
values which are appropriate for the corresponding service version.  This way a user can use
the api to determine the current value.  This is very important for Sahara(Savanna) because
it makes this information available to the user.  Ideally, the stack definition would be self
contained meaning that it contains all default configurations (where possible).

Ok, I will add these back with default values.


- Sid


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


On April 15, 2014, 7:35 p.m., Sid Wagle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20346/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:35 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Sumit Mohanty, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-5387
>     https://issues.apache.org/jira/browse/AMBARI-5387
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> In host overrides we depend on properties defined in {{global.xml}} to determine which
service needs to be restarted when properties are changed. 
> 
> The stack definition API we call however does not provide properties with empty values
in {{global.xml}}. This effects UI because we do not show a service restart required when
necessary.
> 
> http://server:8080/api/v1/stacks2/HDP/versions/2.0.6/stackServices?fields=configurations/StackConfigurations/type
> 
> 
> The specific property was HBase's {{hregion_memstoreflushsize}}.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
52c7309 
>   ambari-server/src/main/java/org/apache/ambari/server/api/util/StackExtensionHelper.java
c31d437 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HBASE/configuration/global.xml
453184b 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HBASE/configuration/hbase-site.xml
bd4d61f 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HDFS/configuration/core-site.xml
d2bff0f 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HDFS/configuration/global.xml
04d51db 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HDFS/configuration/hadoop-policy.xml
900da99 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HDFS/configuration/hdfs-site.xml
9c28bec 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/configuration/global.xml
ae7f586 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/configuration/hive-site.xml
34e6231 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/MAPREDUCE/configuration/global.xml
4633855 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/MAPREDUCE/configuration/mapred-site.xml
7255ca3 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/OOZIE/configuration/global.xml
ddbf780 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/OOZIE/configuration/oozie-site.xml
f4d4b63 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/WEBHCAT/configuration/webhcat-site.xml
16d8691 
>   ambari-server/src/main/resources/stacks/HDP/1.3.2/services/ZOOKEEPER/configuration/global.xml
f78df89 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HBASE/configuration/global.xml
b2c57bd 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HBASE/configuration/hbase-site.xml
cf9416e 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HDFS/configuration/global.xml
ffda6e2 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HDFS/configuration/hdfs-site.xml
d7b37c4 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/configuration/global.xml
ae7f586 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/configuration/hive-site.xml
1e4ba38 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/OOZIE/configuration/global.xml
ddbf780 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/OOZIE/configuration/oozie-site.xml
f96e562 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/WEBHCAT/configuration/webhcat-site.xml
5b5115a 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/YARN/configuration-mapred/global.xml
984cd41 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/YARN/configuration-mapred/mapred-site.xml
310095f 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/YARN/configuration/global.xml
c3a37ef 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/ZOOKEEPER/configuration/global.xml
f78df89 
>   ambari-server/src/main/resources/stacks/HDP/2.1/services/FALCON/configuration/falcon-startup.properties.xml
9746e24 
>   ambari-server/src/main/resources/stacks/HDP/2.1/services/HIVE/configuration/hive-site.xml
e1e3812 
>   ambari-server/src/main/resources/stacks/HDP/2.1/services/OOZIE/configuration/oozie-site.xml
f320b23 
>   ambari-server/src/main/resources/stacks/HDP/2.1/services/WEBHCAT/configuration/webhcat-site.xml
130004a 
>   ambari-server/src/main/resources/stacks/HDP/2.1/services/YARN/configuration/global.xml
9c748da 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
8c0bd65 
>   ambari-server/src/test/java/org/apache/ambari/server/api/util/StackExtensionHelperTest.java
2f17c52 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
73530a8 
>   ambari-web/app/data/HDP2/site_properties.js 3e1c230 
>   ambari-web/app/utils/helper.js 1335ff2 
>   ambari-web/app/views/common/configs/services_config.js 8143711 
> 
> Diff: https://reviews.apache.org/r/20346/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test -Djava.awt.headless=true -DfailIfNoTests=false
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Ambari Main ....................................... SUCCESS [1.903s]
> [INFO] Apache Ambari Project POM ......................... SUCCESS [0.301s]
> [INFO] Ambari Web ........................................ SUCCESS [9.738s]
> [INFO] Ambari Views ...................................... SUCCESS [1.824s]
> [INFO] Ambari Server ..................................... SUCCESS [14:19.226s]
> [INFO] Ambari Agent ...................................... SUCCESS [15.514s]
> [INFO] Ambari Client ..................................... SUCCESS [0.529s]
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Sid Wagle
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message