> On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line30> > > > > Can these states be imported?
No. > On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line209> > > > > Should check that it contains ":" or len is 2 before trying to access > > [0]. No. str() returns string even for None. If it splitted, there is always 0th element b = None str(b).split(":")[0] 'None' > On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line258> > > > > Should close the connection in a finally if it is not None, and open > > inside a try. It's never none. Fixed by placing conn.request() in try-except block, it can throw IO and HTTP exceptions. > On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line300> > > > > Only valid if length of list > 1. the list never has < 2 elements, I do if not metrics or len(metrics) < 2: return (RESULT_STATE_UNKNOWN, ["Unable to calculate the standard deviation for {0} datapoints".format(len(metrics))]) > On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line292> > > > > 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. the list never has < 2 elements, I do if not metrics or len(metrics) < 2: return (RESULT_STATE_UNKNOWN, ["Unable to calculate the standard deviation for {0} datapoints".format(len(metrics))]) renamed method > On Янв. 26, 2016, 1:55 д.п., Alejandro Fernandez wrote: > > 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/diff/1/?file=1219149#file1219149line351> > > > > Not all code paths return a value. This should be consistent. If no explicit return statement, then None returned. Returned None values are handled correctly. - Dmytro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42706/#review116252 ----------------------------------------------------------- On Янв. 25, 2016, 5:28 п.п., Dmytro Sen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42706/ > ----------------------------------------------------------- > > (Updated Янв. 25, 2016, 5:28 п.п.) > > > 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 > >
