> On Nov. 17, 2014, 3:01 p.m., Tom Beerbower wrote: > > Did you consider making alert definitions a sub-resource of alert groups? > > > > So, > > > > > > http://localhost:8080/api/v1/clusters/c1/alert_groups/1/alert_definitions > > > > ... would get all the alert definitions for a specific alert group. > > > > And, > > > > > > http://localhost:8080/api/v1/clusters/c1/alert_groups?fields=alert_definitions > > > > ... would get all the alert definitions for all alert groups. > > > > The advantage of using a sub-resource instead of property is that the > > sub-resource is easier to query. > > > > For example if you wanted to get only the enabled alert definitions for an > > alert group, you could do this with sub-resources ... > > > > > > http://localhost:8080/api/v1/clusters/c1/alert_groups/1/alert_definitions?AlertDefinition/enabled=true > > > > Or if you don't want to return all the fields of the definitions ... > > > > > > http://localhost:8080/api/v1/clusters/c1/alert_groups/1/alert_definitions?fields=AlertDefinition/label > > > > > > With a property you have to get all or none. > > Jonathan Hurley wrote: > I did consider making this a sub resource, but I don't think it's > necessary since there is no requirement to query by AlertDefinition/* when > requesting an AlertGroup; they simply need some properties surfaced from the > alert definition. > > Tom Beerbower wrote: > Ok, as long as you don't think that there is a chance that it will become > a requirement in the future. It would be very strange to have it as both a > property and a sub-resource.
Thanks for the review. That's why I eventually chose a property; because I don't see a use case for it to to be query-able in the future. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28123/#review61779 ----------------------------------------------------------- On Nov. 17, 2014, 3:43 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28123/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2014, 3:43 p.m.) > > > Review request for Ambari, Nate Cole and Tom Beerbower. > > > Bugs: AMBARI-8352 > https://issues.apache.org/jira/browse/AMBARI-8352 > > > Repository: ambari > > > Description > ------- > > When requesting a collection of alert groups via > {{api/v1/clusters/c1/alert_groups}}, the alert definitions that are > associated with that group should be a field that is also returned. > > ``` > http://localhost:8080/api/v1/clusters/c1/alert_groups?fields=AlertGroup/definitions > { > "href" : > "http://localhost:8080/api/v1/clusters/c1/alert_groups?fields=AlertGroup/definitions";, > "items" : [ > { > "href" : "http://localhost:8080/api/v1/clusters/c1/alert_groups/1";, > "AlertGroup" : { > "cluster_name" : "c1", > "definitions" : [ > { > "name" : "ganglia_monitor_mapreduce_history_server", > "label" : "Ganglia History Server Process Monitor", > "enabled" : true, > "service_name" : "GANGLIA", > "component_name" : "GANGLIA_SERVER", > "id" : 1 > }, > { > "name" : "ganglia_monitor_yarn_resourcemanager", > "label" : "Ganglia ResourceManager Process Monitor", > "enabled" : true, > "service_name" : "GANGLIA", > "component_name" : "GANGLIA_SERVER", > "id" : 2 > }, > { > "name" : "ganglia_monitor_hdfs_namenode", > "label" : "Ganglia NameNode Process Monitor", > "enabled" : true, > "service_name" : "GANGLIA", > "component_name" : "GANGLIA_SERVER", > "id" : 3 > }, > { > "name" : "ganglia_monitor_hbase_master", > "label" : "Ganglia HBase Master Process Monitor", > "enabled" : true, > "service_name" : "GANGLIA", > "component_name" : "GANGLIA_SERVER", > "id" : 4 > }, > { > "name" : "ganglia_server_process", > "label" : "Ganglia Server Process", > "enabled" : true, > "service_name" : "GANGLIA", > "component_name" : "GANGLIA_SERVER", > "id" : 5 > } > ], > "id" : 1, > "name" : "GANGLIA" > } > } > ] > } > ``` > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AlertDefinitionResponse.java > 26e2f24 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java > 0c2fb72 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java > 1c85aeb > > Diff: https://reviews.apache.org/r/28123/diff/ > > > Testing > ------- > > New tests added; mvn clean test > > > Thanks, > > Jonathan Hurley > >
