Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-14 Thread Robert Nettleton 

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


Ship it!




Ship It!

- Robert Nettleton


On Feb. 9, 2017, 11:14 p.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> ---
> 
> (Updated Feb. 9, 2017, 11:14 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
> Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Export Blueprints does not contain the settings object and hence the 
> credential store values
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
>  1a9ea91 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
>  1fe48df 
> 
> Diff: https://reviews.apache.org/r/56418/diff/
> 
> 
> Testing
> ---
> 
> Issued 
> http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint
>  command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
>   "recovery_settings" : [
> {
>   "recovery_enabled" : "true"
> }
>   ]
> },
> {
>   "service_settings" : [
> {
>   "name" : "OOZIE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HIVE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HDFS"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "AMBARI_METRICS"
> }
>   ]
> },
> {
>   "component_settings" : [
> {
>   "recovery_enabled" : "true",
>   "name" : "METRICS_COLLECTOR"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "SECONDARY_NAMENODE"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "WEBHCAT_SERVER"
> }
>   ]
> }
>   ]
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-09 Thread Sumit Mohanty 

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


Ship it!




Ship It!

- Sumit Mohanty


On Feb. 9, 2017, 11:14 p.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> ---
> 
> (Updated Feb. 9, 2017, 11:14 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
> Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Export Blueprints does not contain the settings object and hence the 
> credential store values
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
>  1a9ea91 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
>  1fe48df 
> 
> Diff: https://reviews.apache.org/r/56418/diff/
> 
> 
> Testing
> ---
> 
> Issued 
> http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint
>  command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
>   "recovery_settings" : [
> {
>   "recovery_enabled" : "true"
> }
>   ]
> },
> {
>   "service_settings" : [
> {
>   "name" : "OOZIE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HIVE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HDFS"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "AMBARI_METRICS"
> }
>   ]
> },
> {
>   "component_settings" : [
> {
>   "recovery_enabled" : "true",
>   "name" : "METRICS_COLLECTOR"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "SECONDARY_NAMENODE"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "WEBHCAT_SERVER"
> }
>   ]
> }
>   ]
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-09 Thread Madhuvanthi Radhakrishnan 

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

(Updated Feb. 9, 2017, 11:14 p.m.)


Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
Nettleton, and Sumit Mohanty.


Changes
---

Incorporating Review Comments - performance improvement, unit tests


Bugs: AMBARI-19909
https://issues.apache.org/jira/browse/AMBARI-19909


Repository: ambari


Description
---

Export Blueprints does not contain the settings object and hence the credential 
store values


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
 1a9ea91 
  
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
 1fe48df 

Diff: https://reviews.apache.org/r/56418/diff/


Testing
---

Issued 
http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint 
command to ensure that the settings object is present.
Resultant blueprint has the following object added:
"settings" : [
{
  "recovery_settings" : [
{
  "recovery_enabled" : "true"
}
  ]
},
{
  "service_settings" : [
{
  "name" : "OOZIE",
  "credential_store_enabled" : "true"
},
{
  "recovery_enabled" : "true",
  "name" : "HIVE",
  "credential_store_enabled" : "true"
},
{
  "recovery_enabled" : "true",
  "name" : "HDFS"
},
{
  "recovery_enabled" : "true",
  "name" : "AMBARI_METRICS"
}
  ]
},
{
  "component_settings" : [
{
  "recovery_enabled" : "true",
  "name" : "METRICS_COLLECTOR"
},
{
  "recovery_enabled" : "true",
  "name" : "SECONDARY_NAMENODE"
},
{
  "recovery_enabled" : "true",
  "name" : "WEBHCAT_SERVER"
}
  ]
}
  ]


Thanks,

Madhuvanthi Radhakrishnan

Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-08 Thread Jayush Luniya 


> On Feb. 9, 2017, 3:29 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java,
> >  line 136
> > 
> >
> > Madhu,
> > As we discussed, it looks like getting the entire ServiceComponentInfo 
> > is expensive. If I query 
> > /api/v1/clusters//services//components/
> >  it takes a long time. When exporting blueprint since we will be loading 
> > all service components we will have significant perf impact. Lets add 
> > filters to only get required fields for ServiceComponent
> > 
> > 
> > serviceComponentNode.getObject().add("ServiceComponentInfo/cluster_name");
> > 
> > serviceComponentNode.getObject().add("ServiceComponentInfo/service_name");
> > 
> > serviceComponentNode.getObject().add("ServiceComponentInfo/component_name");
> > 
> > serviceComponentNode.getObject().add("ServiceComponentInfo/recovery_enabled");

My suspicion is that the metrics node is taking a long time to load. Will 
follow up on that with Sid.


- Jayush


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


On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> ---
> 
> (Updated Feb. 8, 2017, 12:52 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
> Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Export Blueprints does not contain the settings object and hence the 
> credential store values
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
>  1a9ea91 
> 
> Diff: https://reviews.apache.org/r/56418/diff/
> 
> 
> Testing
> ---
> 
> Issued 
> http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint
>  command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
>   "recovery_settings" : [
> {
>   "recovery_enabled" : "true"
> }
>   ]
> },
> {
>   "service_settings" : [
> {
>   "name" : "OOZIE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HIVE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HDFS"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "AMBARI_METRICS"
> }
>   ]
> },
> {
>   "component_settings" : [
> {
>   "recovery_enabled" : "true",
>   "name" : "METRICS_COLLECTOR"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "SECONDARY_NAMENODE"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "WEBHCAT_SERVER"
> }
>   ]
> }
>   ]
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-08 Thread Jayush Luniya 

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




ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
 (line 136)


Madhu,
As we discussed, it looks like getting the entire ServiceComponentInfo is 
expensive. If I query 
/api/v1/clusters//services//components/
 it takes a long time. When exporting blueprint since we will be loading all 
service components we will have significant perf impact. Lets add filters to 
only get required fields for ServiceComponent


serviceComponentNode.getObject().add("ServiceComponentInfo/cluster_name");

serviceComponentNode.getObject().add("ServiceComponentInfo/service_name");

serviceComponentNode.getObject().add("ServiceComponentInfo/component_name");

serviceComponentNode.getObject().add("ServiceComponentInfo/recovery_enabled");


- Jayush Luniya


On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> ---
> 
> (Updated Feb. 8, 2017, 12:52 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
> Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Export Blueprints does not contain the settings object and hence the 
> credential store values
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
>  1a9ea91 
> 
> Diff: https://reviews.apache.org/r/56418/diff/
> 
> 
> Testing
> ---
> 
> Issued 
> http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint
>  command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
>   "recovery_settings" : [
> {
>   "recovery_enabled" : "true"
> }
>   ]
> },
> {
>   "service_settings" : [
> {
>   "name" : "OOZIE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HIVE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HDFS"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "AMBARI_METRICS"
> }
>   ]
> },
> {
>   "component_settings" : [
> {
>   "recovery_enabled" : "true",
>   "name" : "METRICS_COLLECTOR"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "SECONDARY_NAMENODE"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "WEBHCAT_SERVER"
> }
>   ]
> }
>   ]
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 56418: Export Blueprints does not contain the settings object and hence the credential store values

2017-02-08 Thread Robert Nettleton 

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


Fix it, then Ship it!




The code changes in this patch look fine to me, but I'd like to request two 
things:

1. Unit tests for this change be added to the patch
2. Some additional manual testing, mentioned below, to verify that the exported 
Blueprint will deploy properly. 

Thanks for providing this patch!


ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
 (line 75)


Overall, the changes look fine to me, but could you please add some unit 
tests for this change?  

If you could add some tests to the following:

org.apache.ambari.server.api.query.render.ClusterBlueprintRendererTest

that would be great.  

Thanks.



ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
 (line 76)


Could you also please try out some manual tests to use the exported 
Blueprint to create a new cluster? 

When changes to the export renderer are made, its always a good thing to 
try to make sure that the exported Blueprint is valid, and can be used for a 
successful cluster creation. 

Thanks.


- Robert Nettleton


On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> ---
> 
> (Updated Feb. 8, 2017, 12:52 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert 
> Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Export Blueprints does not contain the settings object and hence the 
> credential store values
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java
>  1a9ea91 
> 
> Diff: https://reviews.apache.org/r/56418/diff/
> 
> 
> Testing
> ---
> 
> Issued 
> http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint
>  command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
>   "recovery_settings" : [
> {
>   "recovery_enabled" : "true"
> }
>   ]
> },
> {
>   "service_settings" : [
> {
>   "name" : "OOZIE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HIVE",
>   "credential_store_enabled" : "true"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "HDFS"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "AMBARI_METRICS"
> }
>   ]
> },
> {
>   "component_settings" : [
> {
>   "recovery_enabled" : "true",
>   "name" : "METRICS_COLLECTOR"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "SECONDARY_NAMENODE"
> },
> {
>   "recovery_enabled" : "true",
>   "name" : "WEBHCAT_SERVER"
> }
>   ]
> }
>   ]
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>