Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-20 Thread Robert Levas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44916/ --- (Updated March 16, 2016, 4:55 p.m.) Review request for Ambari, Dmytro Sen,

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Sid Wagle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44916/#review123895 --- Shouldn't we also have a non-null check in the backend code? -

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Robert Levas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44916/ --- (Updated March 17, 2016, 1:48 p.m.) Review request for Ambari, Dmytro Sen,

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Jonathan Hurley
> On March 16, 2016, 5:14 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java, > > line 224 > > > > > > How can this

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Robert Levas
> On March 17, 2016, 8:53 a.m., Jonathan Hurley wrote: > > My only concern is for safety and clarify of code, does it makes sense to > > change the contract of the config to always return a non-null (if empty) > > map? I attempted to go this route, but I got stuck where the Jackson JSON

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Robert Levas
> On March 16, 2016, 5:14 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java, > > line 224 > > > > > > How can this

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-19 Thread Robert Levas
> On March 16, 2016, 1:57 p.m., Sid Wagle wrote: > > Shouldn't we also have a non-null check in the backend code? I am not sure.. I guess it depends on the expectations. Historically it appears that the property map is expected to not be null. I am happy to add null protections in place I

Re: Review Request 44916: StackAdvisor needs to set config map for ams-grafana-env to meet non-null expectations

2016-03-18 Thread Jonathan Hurley
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44916/#review123930 ---