> On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java, > > lines 50-63 > > <https://reviews.apache.org/r/27790/diff/1/?file=755705#file755705line50> > > > > Why change to public here, I didn't see any cases outside the classes > > of using it > > Jonathan Hurley wrote: > The renderer uses some of them: > properties.add(AlertResourceProvider.ALERT_STATE); > > I can make only the exposed ones public.
Ah, ok, didn't see that one (I only searched for ALERT_CLUSTER_NAME etc) > On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java, > > lines 45-47 > > <https://reviews.apache.org/r/27790/diff/1/?file=755704#file755704line45> > > > > Don't need this doc annotation since it's implied and already using the > > @Override? > > Jonathan Hurley wrote: > I love being explicit. Also, this allows us to have proper javadoc on all > methods creating nice visual separation in the file. I was dinged in my early days for unneccessary doc. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27790/#review60599 ----------------------------------------------------------- On Nov. 9, 2014, 9:14 a.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27790/ > ----------------------------------------------------------- > > (Updated Nov. 9, 2014, 9:14 a.m.) > > > Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower. > > > Bugs: AMBARI-8237 > https://issues.apache.org/jira/browse/AMBARI-8237 > > > Repository: ambari > > > Description > ------- > > The web client would like to be able to request a customizable alert summary > structure given a combination of parameters, such as alert definition name, > host, date, etc. Currently the alerts_summary structure off of the > cluster/service/host endpoints are static and just return total counts. > > The new structure would also need to contain some extra information, such as > original timestamp when the most recent state change occurred: > > "alerts_summary" : { > "CRITICAL" : { > “count”: 2, > “original_timestamp”: 1415134996589 > }, > "OK” : { > “count”: 45, > “original_timestamp”: 1415134133489 > } > ... > } > > The Ambari API already has a model to model in cases like this. We use a > "renderer" which is a value that instructs the API engine to format the > results of a query in a particular fashion. With this, we can query the > alerts endpoint and format it for a summary output. This includes formatting > for alerts by name, host, and other supported combinations. > > Some URI examples: > http://localhost:8080/api/v1/clusters/c1/alerts?format=summary > http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java > d7aca22 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java > 715d017 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java > ef014a9 > > Diff: https://reviews.apache.org/r/27790/diff/ > > > Testing > ------- > > New tests added to ensure the renderer converts the flattened alert data > correctly. > > > Thanks, > > Jonathan Hurley > >