Changes for code review

data/database.py

@@ -892,6 +892,13 @@ class BUILD_PHASE(object):
             phase == cls.CANCELLED)
 
 
+class TRIGGER_DISABLE_REASON(object):
+  """ Build trigger disable reason enum """
+  BUILD_FALURES = 'successive_build_failures'
+  INTERNAL_ERRORS = 'successive_build_internal_errors'
+  USER_TOGGLED = 'user_toggled'
+
+
 class QueueItem(BaseModel):
   queue_name = CharField(index=True, max_length=1024)
   body = TextField()

data/migrations/versions/61cadbacb9fc_add_ability_for_build_triggers_to_be_.py

@@ -1,24 +1,25 @@
 """Add ability for build triggers to be disabled
 
 Revision ID: 61cadbacb9fc
-Revises: d8989249f8f6
+Revises: b4c2d45bc132
 Create Date: 2017-10-18 12:07:26.190901
 
 """
 
 # revision identifiers, used by Alembic.
 revision = '61cadbacb9fc'
-down_revision = 'd8989249f8f6'
+down_revision = 'b4c2d45bc132'
 
 from alembic import op
 import sqlalchemy as sa
 from sqlalchemy.dialects import mysql
+from util.migrate import UTF8CharField
 
 def upgrade(tables):
     # ### commands auto generated by Alembic - please adjust! ###
     op.create_table('disablereason',
     sa.Column('id', sa.Integer(), nullable=False),
-    sa.Column('name', sa.String(length=255), nullable=False),
+    sa.Column('name', UTF8CharField(length=255), nullable=False),
     sa.PrimaryKeyConstraint('id', name=op.f('pk_disablereason'))
     )
     op.create_index('disablereason_name', 'disablereason', ['name'], unique=True)

data/model/build.py

@@ -7,7 +7,7 @@ from peewee import JOIN_LEFT_OUTER
 import features
 from data.database import (BuildTriggerService, RepositoryBuildTrigger, Repository, Namespace, User,
                            RepositoryBuild, BUILD_PHASE, db_random_func, UseThenDisconnect,
-                           DisableReason)
+                           DisableReason, TRIGGER_DISABLE_REASON)
 from data.model import (InvalidBuildTriggerException, InvalidRepositoryBuildException,
                         db_transaction, user as user_model, config)
 
@@ -258,12 +258,13 @@ def mark_build_archived(build_uuid):
           .execute()) > 0
 
 
-def toggle_build_trigger(trigger, enabled, reason='user_toggled'):
+def toggle_build_trigger(trigger, enabled, reason=TRIGGER_DISABLE_REASON.USER_TOGGLED):
   """ Toggles the enabled status of a build trigger. """
   trigger.enabled = enabled
 
   if not enabled:
     trigger.disabled_reason = RepositoryBuildTrigger.disabled_reason.get_id(reason)
+    trigger.disabled_datetime = datetime.utcnow()
 
   trigger.save()
 
@@ -275,6 +276,12 @@ def update_trigger_disable_status(trigger, final_phase):
       ensures that triggers that continue to error are eventually automatically disabled.
   """
   with db_transaction():
+    try:
+      trigger = RepositoryBuildTrigger.get(id=trigger.id)
+    except RepositoryBuildTrigger.DoesNotExist:
+      # Already deleted.
+      return
+
     # If the build completed successfully, then reset the successive counters.
     if final_phase == BUILD_PHASE.COMPLETE:
       trigger.successive_failure_count = 0
@@ -287,7 +294,6 @@ def update_trigger_disable_status(trigger, final_phase):
       trigger.successive_failure_count = trigger.successive_failure_count + 1
       trigger.successive_internal_error_count = 0
     elif final_phase == BUILD_PHASE.INTERNAL_ERROR:
-      trigger.successive_failure_count = 0
       trigger.successive_internal_error_count = trigger.successive_internal_error_count + 1
 
     # Check if we need to disable the trigger.    
@@ -295,15 +301,11 @@ def update_trigger_disable_status(trigger, final_phase):
     error_threshold = config.app_config.get('SUCCESSIVE_TRIGGER_INTERNAL_ERROR_DISABLE_THRESHOLD')
 
     if failure_threshold and trigger.successive_failure_count >= failure_threshold:
-      trigger.disabled_datetime = datetime.utcnow()
+      toggle_build_trigger(trigger, False, TRIGGER_DISABLE_REASON.BUILD_FALURES)
-      trigger.enabled = False
-      trigger.disabled_reason = RepositoryBuildTrigger.disabled_reason.get_id('successive_build_failures')
     elif (error_threshold and
           trigger.successive_internal_error_count >= error_threshold):
-      trigger.disabled_datetime = datetime.utcnow()
+      toggle_build_trigger(trigger, False, TRIGGER_DISABLE_REASON.INTERNAL_ERRORS)
-      trigger.enabled = False
+    else:
-      trigger.disabled_reason = RepositoryBuildTrigger.disabled_reason.get_id('successive_build_internal_errors')
-
       # Save the trigger changes.
       trigger.save()