> On Nov. 18, 2014, 9:09 a.m., Nate Cole wrote: > > ambari-server/src/main/resources/Ambari-DDL-SQLServer-DROP.sql, lines > > 171-172 > > <https://reviews.apache.org/r/28159/diff/1/?file=766864#file766864line171> > > > > Since there's a FK, I'm not sure if this needs to be removed before > > alert_target. I don't understand SQLServer enough to know > > Jonathan Hurley wrote: > I think you're right; dropping it before alert_target. > > Also, I'm not sure that this is the best way to drop tables in SQLServer; > I'd think they could do a query to get the tables that they'd need to drop.
+1 to that. > On Nov. 18, 2014, 9:09 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java, > > lines 328-331 > > <https://reviews.apache.org/r/28159/diff/1/?file=766855#file766855line328> > > > > This is an assumption that a caller may not realize (an empty list > > means "do them all"). Consider an Exception instead for those who would do > > ill. > > Jonathan Hurley wrote: > Thanks for the review and a nice point to mention. > > Jeff and I spoke about this exact issue and we felt that since this is an > optional parameter, neglecting to specify it should have the same effect as > what would have happened before this change went in. Not specifying the value > should mean that the target is a regular target, interested in all alert > states. Very well :) - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28159/#review61920 ----------------------------------------------------------- On Nov. 18, 2014, 9:16 a.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28159/ > ----------------------------------------------------------- > > (Updated Nov. 18, 2014, 9:16 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Nate Cole, and Tom Beerbower. > > > Bugs: AMBARI-8362 > https://issues.apache.org/jira/browse/AMBARI-8362 > > > Repository: ambari > > > Description > ------- > > Alert targets should have an optional field that represents the severity > levels that they care about. This will allow users to create different > targets that are only alerted when an alert's state has transitioned to their > support criticality level. > > For example, a user may want to have 2 different email targets, "Tier-1 > Support" and "Tier-2 Support" where T1 receives WARNINGs only and T2 receives > CRITICALS and UNKNOWNS. > > We will extend the AlertTarget resource to provide this extra option. It will > be a list of supported criticality levels. If an empty list is specified, all > alert states will be accepted for the target. > > An example of creating a target that only cares about OK and WARNING states. > ``` > { > "AlertTarget": { > "name": "Administrators", > "description": "The Admins", > "notification_type": "EMAIL", > "alert_states": ["OK", "WARNING"], > "properties":{ > "foo": "bar", > "foobar" : "baz" > } > } > } > ``` > > There was a database design choice that I had to make WRT storing a Set of > enumerations. JPA actually has a mechanism for this, but since Ambari doesn't > use JPA correctly, it involves creating a new table by hand. The other > options that I considered were: > - JSON or CSV string of the enumerations > - A bit representing the OR'd value > - A BLOB column that stored the serialized set > > I chose the table since it allows us to leverage JPA for the persistence and > retrieval while preventing our providers or DAOs from needing specialized > serialization logic. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java > 3029114 > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/AlertStateChangedListener.java > c42851b > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertTargetEntity.java > 12c394d > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog200.java > 2cbf266 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 4d15914 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql e3ae87a > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 0587232 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 5605d57 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 79caca7 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-DROP.sql 7658b63 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java > 7a633e7 > > ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertStateChangedEventTest.java > 18b4123 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog200Test.java > be97222 > > Diff: https://reviews.apache.org/r/28159/diff/ > > > Testing > ------- > > mvn clean test and manual testing to ensure that the alert targets are > skipped and no notices are created. > > > Thanks, > > Jonathan Hurley > >
