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

[Jonathan Hurley]

> On March 10, 2017, 10:40 a.m., Jonathan Hurley wrote:
> > Ship It!
> 
> Anita Jebaraj wrote:
> Thank you Jonathan for reviewing, I had to update the patch since there 
> was an unnecessarily check lingering around in the patch, 
> +if(propertiesToHideInResponse != null) { 
> this wont be required now since its assigned to an empty set on the 
> invocation of the method. Can you please check this out and help me in 
> pushing the changes to trunk

Done. Please close this review.


- Jonathan


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


On March 10, 2017, 10:34 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 10, 2017, 10:34 a.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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
>  8111a39 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/8/
> 
> 
> 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

[Anita Jebaraj]

> On March 10, 2017, 3:40 p.m., Jonathan Hurley wrote:
> > Ship It!

Thank you Jonathan for reviewing, I had to update the patch since there was an 
unnecessarily check lingering around in the patch, 
+if(propertiesToHideInResponse != null) { 
this wont be required now since its assigned to an empty set on the invocation 
of the method. Can you please check this out and help me in pushing the changes 
to trunk


- Anita


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


On March 10, 2017, 3:34 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 10, 2017, 3:34 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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
>  8111a39 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/8/
> 
> 
> 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

[Jonathan Hurley]

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


Ship it!




Ship It!

- Jonathan Hurley


On March 10, 2017, 10:34 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 10, 2017, 10:34 a.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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
>  8111a39 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/7/
> 
> 
> 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

[Anita Jebaraj]

> On March 8, 2017, 6:44 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 4314-4336 (patched)
> > 
> >
> > This reads from the stream on every invocation. Since it's called a 
> > REST endpoint, that means the response is based on disk I/O ... can we just 
> > load these once on startup like other configuration properties?

This is fixed in the new patch


- Anita


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


On March 10, 2017, 3:34 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 10, 2017, 3:34 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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
>  8111a39 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/7/
> 
> 
> 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

[Anita Jebaraj]

> On March 10, 2017, 2:05 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 4356 (patched)
> > 
> >
> > Can you initialize this on the first invocation of this method? That 
> > way, if the black list file isn't defined, it doesn't have to go through 
> > the check over and over.

please refer to the new patch


- Anita


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


On March 10, 2017, 3:34 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 10, 2017, 3:34 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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
>  8111a39 
> 
> 
> Diff: https://reviews.apache.org/r/57168/diff/7/
> 
> 
> 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

[Anita Jebaraj]

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

(Updated March 10, 2017, 3:34 p.m.)


Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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 (updated)
-

  ambari-server/docs/configuration/index.md af962e1 
  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 df334c5 
  
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
 8111a39 


Diff: https://reviews.apache.org/r/57168/diff/7/

Changes: https://reviews.apache.org/r/57168/diff/6-7/


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

[Jonathan Hurley]

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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4356 (patched)


Can you initialize this on the first invocation of this method? That way, 
if the black list file isn't defined, it doesn't have to go through the check 
over and over.


- Jonathan Hurley


On March 8, 2017, 6:30 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 8, 2017, 6:30 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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/docs/configuration/index.md af962e1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  df334c5 
>   
> 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/6/
> 
> 
> 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

[Anita Jebaraj]

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

(Updated March 8, 2017, 11:30 p.m.)


Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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 (updated)
-

  ambari-server/docs/configuration/index.md af962e1 
  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 df334c5 
  
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/6/

Changes: https://reviews.apache.org/r/57168/diff/5-6/


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

[Jonathan Hurley]

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



Since you're adding a new property, can you run the markdown generator:

mvn exec:java
or
mvn exec:java@configuration-markdown (on maven 3.3.1 and later)


ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4314-4336 (patched)


This reads from the stream on every invocation. Since it's called a REST 
endpoint, that means the response is based on disk I/O ... can we just load 
these once on startup like other configuration properties?


- Jonathan Hurley


On March 8, 2017, noon, Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 8, 2017, noon)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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
>  0991814 
>   
> 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/5/
> 
> 
> 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

[Sangeeta Ravindran]

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


Ship it!




Ship It!

- Sangeeta Ravindran


On March 8, 2017, 5 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 8, 2017, 5 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jaimin Jetly, Jonathan Hurley, 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
>  0991814 
>   
> 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/5/
> 
> 
> 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

[Di Li]

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


Ship it!




Ship It!

- Di Li


On March 6, 2017, 10:45 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 6, 2017, 10:45 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/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  0991814 
>   
> 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/5/
> 
> 
> 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

[Anita Jebaraj]

> On March 6, 2017, 9:50 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
> > Lines 165 (patched)
> > 
> >
> > The for loop should not run anyway when propertiesToHideInResponse is 
> > empty.

Please review the new patch. Thank you


- Anita


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


On March 6, 2017, 10:45 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 6, 2017, 10:45 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/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  0991814 
>   
> 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/5/
> 
> 
> 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

[Anita Jebaraj]

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

(Updated March 6, 2017, 10:45 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 (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 0991814 
  
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/5/

Changes: https://reviews.apache.org/r/57168/diff/4-5/


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

[Di Li]

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




ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4328 (patched)


This should be an error. Use String.format instead of "+".



ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
Lines 165 (patched)


The for loop should not run anyway when propertiesToHideInResponse is empty.


- Di Li


On March 3, 2017, 8:47 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 3, 2017, 8:47 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/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/4/
> 
> 
> 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

[Anita Jebaraj]

> On March 3, 2017, 3:59 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 4324 (patched)
> > 
> >
> > Why did you need to restore the keys ? stringPropertyNames already 
> > returns a Set object that you may use directly.

Yes...I can use set directly, I have updated the patch


> On March 3, 2017, 3:59 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 4327 (patched)
> > 
> >
> > close instream in finally{}

Fixed in new patch


> On March 3, 2017, 3:59 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 4329 (patched)
> > 
> >
> > How did Ambari server react to an i/o exception on the blacklist file? 
> > Please make sure Ambari server start restarts successfully even when the 
> > blacklist file causes an i/o exception as I don't think it's a sev1 err 
> > like some others in this class, i.e. can't read db pwd, can't read ssl cert 
> > pwd.

Throwing the exception would cause server error, so I have changed it to print 
a message in the log


> On March 3, 2017, 3:59 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
> > Lines 168 (patched)
> > 
> >
> > The second "tab" seemed to be a real tab than 2 white spaces. please 
> > make sure all tabs are replaced by spaces. You can set that in Eclipse as a 
> > general setting.

Fixed in new patch


- Anita


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


On March 3, 2017, 8:47 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57168/
> ---
> 
> (Updated March 3, 2017, 8:47 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/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/4/
> 
> 
> 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

[Anita Jebaraj]

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

(Updated March 3, 2017, 8:47 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 (updated)
-

  
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/4/

Changes: https://reviews.apache.org/r/57168/diff/3-4/


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

[Di Li]

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




ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4324 (patched)


Why did you need to restore the keys ? stringPropertyNames already returns 
a Set object that you may use directly.



ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4327 (patched)


close instream in finally{}



ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 4329 (patched)


How did Ambari server react to an i/o exception on the blacklist file? 
Please make sure Ambari server start restarts successfully even when the 
blacklist file causes an i/o exception as I don't think it's a sev1 err like 
some others in this class, i.e. can't read db pwd, can't read ssl cert pwd.



ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
Lines 168 (patched)


The second "tab" seemed to be a real tab than 2 white spaces. please make 
sure all tabs are replaced by spaces. You can set that in Eclipse as a general 
setting.


- Di Li


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
> 
>

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

[Anita Jebaraj]

> On March 1, 2017, 4:28 p.m., Di Li wrote:
> > ambari-server/conf/unix/ambari-blacklist.properties
> > Lines 1 (patched)
> > 
> >
> > 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
> 
>

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

[Alejandro Fernandez]

> On March 1, 2017, 6:57 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
> > Lines 164 (patched)
> > 
> >
> > Is this because of some sort of security concern?
> > Why hide properties from the API?
> > 
> > If you do want to hide props, perhaps it's best added as a new 
> > [blacklisted] section to the current file so that both files can be kept 
> > in-sync.
> 
> Anita Jebaraj wrote:
> Hi Alejandro, hiding the properties from the API would help the users to 
> remove the properties which has information such as database name, database 
> user name etc,  that are not used in the ambari-web, but exposed in the API. 
> 
> I would like to follow the approach suggested by Di to create a property 
> in ambari.properties which would point to the file containing the list that 
> needs to blacklisted. Eventually if there are many properties that are 
> blacklisted, adding a section [blacklisted] would cause the ambari.properties 
> file to grow and make it difficult to maintain. Once the jira is accepted I 
> can write a wiki about how to use this feature.

I'm ok with this approach. Thank you for explaining


- Alejandro


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


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
> 
>

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

[Anita Jebaraj]

> On March 1, 2017, 6:57 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/RootServiceResponseFactory.java
> > Lines 164 (patched)
> > 
> >
> > Is this because of some sort of security concern?
> > Why hide properties from the API?
> > 
> > If you do want to hide props, perhaps it's best added as a new 
> > [blacklisted] section to the current file so that both files can be kept 
> > in-sync.

Hi Alejandro, hiding the properties from the API would help the users to remove 
the properties which has information such as database name, database user name 
etc,  that are not used in the ambari-web, but exposed in the API. 

I would like to follow the approach suggested by Di to create a property in 
ambari.properties which would point to the file containing the list that needs 
to blacklisted. Eventually if there are many properties that are blacklisted, 
adding a section [blacklisted] would cause the ambari.properties file to grow 
and make it difficult to maintain. Once the jira is accepted I can write a wiki 
about how to use this feature.


- Anita


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


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
> 
>

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

[Anita Jebaraj]

---
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 (updated)
-

  
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/

Changes: https://reviews.apache.org/r/57168/diff/2-3/


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

[Anita Jebaraj]

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

(Updated Feb. 28, 2017, 8:59 p.m.)


Review request for Ambari, Di Li, Oleksandr Diachenko, and Sangeeta Ravindran.


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 (updated)
-

  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/


Testing
---

Added 1 test case
Ran mvn test


Thanks,

Anita Jebaraj

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

[Anita Jebaraj]

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

Review request for Ambari, Di Li, Oleksandr Diachenko, and Sangeeta Ravindran.


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/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/


Testing
---

Added 1 test case
Ran mvn test


Thanks,

Anita Jebaraj