----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42706/#review116252 -----------------------------------------------------------
ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 30) <https://reviews.apache.org/r/42706/#comment177264> Can these states be imported? ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 102) <https://reviews.apache.org/r/42706/#comment177263> Can use shorthand notation for these, x = dictionary[key] if key in dictionary else default_value ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 191) <https://reviews.apache.org/r/42706/#comment177265> Can just replace "http" for "https" if ssl is enabled. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 209) <https://reviews.apache.org/r/42706/#comment177266> Should check that it contains ":" or len is 2 before trying to access [0]. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 258) <https://reviews.apache.org/r/42706/#comment177268> Should close the connection in a finally if it is not None, and open inside a try. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 276) <https://reviews.apache.org/r/42706/#comment177269> If the mean is 0, can't divide by 0. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 292) <https://reviews.apache.org/r/42706/#comment177271> Technically, this is the std deviation of a sample and not the entire collection since it divides by len(lst) - 1 instead of just len(lst). Should perhaps clarify that in the function signature. Should check if list is empty or of size 1 to avoid division by negative numbers of 0. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 300) <https://reviews.apache.org/r/42706/#comment177272> Only valid if length of list > 1. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 323) <https://reviews.apache.org/r/42706/#comment177273> Not all code paths return a value. If an exception is thrown, nothing is returned. ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py (line 351) <https://reviews.apache.org/r/42706/#comment177274> Not all code paths return a value. This should be consistent. - Alejandro Fernandez On Jan. 25, 2016, 5:28 p.m., Dmytro Sen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42706/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2016, 5:28 p.m.) > > > Review request for Ambari, Jonathan Hurley and Sid Wagle. > > > Repository: ambari > > > Description > ------- > > Implement script alert based on AMS metrics standard deviation > > > Diffs > ----- > > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/alerts/alert_metrics_deviation.py > PRE-CREATION > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json > 1eda00f > > Diff: https://reviews.apache.org/r/42706/diff/ > > > Testing > ------- > > Unit tests passed > > > Thanks, > > Dmytro Sen > >
