You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Di Li <di...@ca.ibm.com> on 2017/03/01 16:28:05 UTC

Re: Review Request 57168: Include option to filter out properties from APi that returns ambari.properties file

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




ambari-server/conf/unix/ambari-blacklist.properties
Lines 1 (patched)
<https://reviews.apache.org/r/57168/#comment239409>

    I am not sure Ambari should ship with empty files.
    
    I would suggest you to add an optional property to ambari.property that points to the blacklist properties file.
    
    Let's say we call it property.mask.file, its value should be the absolute path of a properties file.
    
    Logic can be:
    If "property.mask.file" does not exist in ambari.properties - no action
    If "property.mask.file" does not point to an existing property file - print some log but no further action
    Else - mask
    
    Write a wiki on Apache Ambari wiki on how to use this optional property.



ambari-server/conf/windows/ambari-blacklist.properties
Lines 1 (patched)
<https://reviews.apache.org/r/57168/#comment239410>

    I am not sure Ambari should ship with empty files.
    
    I would suggest you to add an optional property to ambari.property that points to the blacklist properties file.
    
    Let's say we call it property.mask.file, its value should be the absolute path of a properties file.
    
    Logic can be:
    If "property.mask.file" does not exist in ambari.properties - no action
    If "property.mask.file" does not point to an existing property file - print some log but no further action
    Else - mask
    
    Write a wiki on Apache Ambari wiki on how to use this optional property.



ambari-server/src/main/assemblies/server-windows.xml
Lines 44 (patched)
<https://reviews.apache.org/r/57168/#comment239412>

    please refer to my comments about not shipping empty files



ambari-server/src/main/assemblies/server.xml
Lines 257 (patched)
<https://reviews.apache.org/r/57168/#comment239411>

    please refer to my comments about not shipping empty files


- Di Li


On Feb. 28, 2017, 9:02 p.m., Anita Jebaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 9:02 p.m.)
> 
> 
> Review request for Ambari, Di Li, Oleksandr Diachenko, Sangeeta Ravindran, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-20243
>     https://issues.apache.org/jira/browse/AMBARI-20243
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently all the details from the ambari.properties file is being returned by the API call.
> 
> Some of those information may not be utilized and hence an option can be provided to filter the properties
> 
> A ambari-blacklist.properties file can be created, the properties that are entered in the file, will be removed from the api call that returns the ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/ambari-blacklist.properties PRE-CREATION 
>   ambari-server/conf/windows/ambari-blacklist.properties PRE-CREATION 
>   ambari-server/src/main/assemblies/server-windows.xml ce1e270 
>   ambari-server/src/main/assemblies/server.xml cc9ad0f 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java eaecf35 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java dadcf09 
>   ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java 51114f8 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/2/
> 
> 
> Testing
> -------
> 
> Added 1 test case
> Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>


Re: Review Request 57168: Include option to filter out properties from APi that returns ambari.properties file

Posted by Anita Jebaraj <aj...@us.ibm.com>.

> On March 1, 2017, 4:28 p.m., Di Li wrote:
> > ambari-server/conf/unix/ambari-blacklist.properties
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/57168/diff/2/?file=1652014#file1652014line1>
> >
> >     I am not sure Ambari should ship with empty files.
> >     
> >     I would suggest you to add an optional property to ambari.property that points to the blacklist properties file.
> >     
> >     Let's say we call it property.mask.file, its value should be the absolute path of a properties file.
> >     
> >     Logic can be:
> >     If "property.mask.file" does not exist in ambari.properties - no action
> >     If "property.mask.file" does not point to an existing property file - print some log but no further action
> >     Else - mask
> >     
> >     Write a wiki on Apache Ambari wiki on how to use this optional property.

I have updated the patch as per your suggestions..Please review the new changes


- Anita


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


On March 2, 2017, 1:05 a.m., Anita Jebaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> -----------------------------------------------------------
> 
> (Updated March 2, 2017, 1:05 a.m.)
> 
> 
> Review request for Ambari, Di Li, Oleksandr Diachenko, Sangeeta Ravindran, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-20243
>     https://issues.apache.org/jira/browse/AMBARI-20243
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently all the details from the ambari.properties file is being returned by the API call.
> 
> Some of those information may not be utilized and hence an option can be provided to filter the properties
> 
> A ambari-blacklist.properties file can be created, the properties that are entered in the file, will be removed from the api call that returns the ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java eaecf35 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java dadcf09 
>   ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java 51114f8 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/3/
> 
> 
> Testing
> -------
> 
> Added 1 test case
> Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>