You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2014/11/17 19:17:17 UTC
Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28123/
-----------------------------------------------------------
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
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28123/#review61814
-----------------------------------------------------------
Ship it!
Ship It!
- Tom Beerbower
On Nov. 17, 2014, 8: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, 8: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
>
>
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
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 (updated)
-----
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
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Nov. 17, 2014, 8: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.
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.
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28123/#review61779
-----------------------------------------------------------
On Nov. 17, 2014, 8: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, 8: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
>
>
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> 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
>
>
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> 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.
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.
> On Nov. 17, 2014, 3:01 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java, line 400
> > <https://reviews.apache.org/r/28123/diff/1/?file=765939#file765939line400>
> >
> > I think that this check would fail if the caller asked for '?fields=AlertGroup', which should include evertything in that category.
> >
> > You could instead use BaseProvider.isPropertyRequested(String propertyId, Set<String> requestedIds).
Fixed, updating a new patch.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28123/#review61779
-----------------------------------------------------------
On Nov. 17, 2014, 1:17 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, 1:17 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
>
>
Re: Review Request 28123: Alert Groups REST Endpoint Should Support
Associated Definitions
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28123/#review61779
-----------------------------------------------------------
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.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java
<https://reviews.apache.org/r/28123/#comment103669>
I think that this check would fail if the caller asked for '?fields=AlertGroup', which should include evertything in that category.
You could instead use BaseProvider.isPropertyRequested(String propertyId, Set<String> requestedIds).
- Tom Beerbower
On Nov. 17, 2014, 6:17 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, 6:17 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
>
>