-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45719/#review127002
-----------------------------------------------------------


Fix it, then Ship it!





ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
 (line 58)
<https://reviews.apache.org/r/45719/#comment190133>

    You may call it [Alert HAWQ]
    
    Would be great help for **grep**-pers out there.



ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
 (line 74)
<https://reviews.apache.org/r/45719/#comment190135>

    else is not necessary, since the previous line has a return statement



ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
 (line 77)
<https://reviews.apache.org/r/45719/#comment190136>

    This message would be too long for the user to see on the Ambari UI.
    
    If I were adding this message, I would point them to 
gp_segment_configuration table since that is the originial source of 
information. Plus, the table has more information than the log message we dump 
into the alerts log.



ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
 (line 101)
<https://reviews.apache.org/r/45719/#comment190134>

    You do not have to use strip() again - you have used strip in the previous 
line to remove trailing whitespaces.



ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
 (line 110)
<https://reviews.apache.org/r/45719/#comment190132>

    Would the file be closed after execution of this line?
    
    Better to use 'with open(HAWQ_SLAVES_FILE) as f', so that you can ensure 
that the file is closed and frees up system resources.


- Matt


On April 4, 2016, 5:41 p.m., Goutam Tadi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45719/
> -----------------------------------------------------------
> 
> (Updated April 4, 2016, 5:41 p.m.)
> 
> 
> Review request for Ambari, Alexander Denissov, bhuvnesh chaudhary, Lav Jain, 
> and Matt.
> 
> 
> Bugs: AMBARI-15704
>     https://issues.apache.org/jira/browse/AMBARI-15704
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Include an alert informing the number of segments marked down in 
> gp_segment_configuration table
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/resources/common-services/HAWQ/2.0.0/alerts.json 
> 8da5beb 
>   
> ambari-server/src/main/resources/common-services/HAWQ/2.0.0/package/alerts/alert_segment_registration_status.py
>  PRE-CREATION 
>   
> ambari-server/src/test/python/stacks/2.3/HAWQ/test_alert_segment_registration_status.py
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45719/diff/
> 
> 
> Testing
> -------
> 
> Test cases added.
> 
> test_missing_configs 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> test_missing_slave_file 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> test_successful_registration_status 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> test_unsuccessful_empty_db_registration_status 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> test_unsuccessful_registration_status 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> test_unsuccessful_registration_status_plural 
> (test_alert_segment_registration_status.TestAlertRegistrationStatus) ... ok
> 
> 
> Thanks,
> 
> Goutam Tadi
> 
>

Reply via email to