Exception handling in draining thread (#16)

Azure · May 26, 2021 · 24472f3 · 24472f3
1 parent 355898a
commit 24472f3
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 23 deletions.
diff --git a/src/gitops_connector.py b/src/gitops_connector.py
@@ -55,19 +55,27 @@ def notify_abandoned_pr_tasks(self):
     # in the order of the request received time.
     def drain_commit_status_queue(self):
         while (True):
-            # Blocking get
-            commit_status = self._global_message_queue.get()
+            try:
+                # Blocking get
+                commit_status = self._global_message_queue.get()
 
-            if not commit_status:
-                break
+                if not commit_status:
+                    break
 
-            # Queue entry is (received time, commit_status)
-            commit_status = commit_status[1]
+                # Queue entry is (received time, commit_status)
+                commit_status = commit_status[1]
 
-            self._git_repository.post_commit_status(commit_status)
+                # Handling an exception as it crashes the draining thread
+                try:
+                    self._git_repository.post_commit_status(commit_status)
 
-            for subscriber in self._raw_subscribers:
-                subscriber.post_commit_status(commit_status)
+                    for subscriber in self._raw_subscribers:
+                        subscriber.post_commit_status(commit_status)
+                except Exception as e:
+                    logging.error(f'Failed to update GitCommit Status: {e}')
+
+            except Exception as e:
+                logging.error(f'Unexpected exception in the message queue draining thread: {e}')
 
 
 if __name__ == "__main__":

diff --git a/src/operators/flux_gitops_operator.py b/src/operators/flux_gitops_operator.py
@@ -7,8 +7,8 @@
 from operators.git_commit_status import GitCommitStatus
 
 KUSTOMIZATION_PHASE = "Kustomization"
-PROGRESSING_SATE = "Progressing"
-HEALTH_CHECK_FAILED_SATE = "HealthCheckFailed"
+PROGRESSING_STATE = "Progressing"
+HEALTH_CHECK_FAILED_STATE = "HealthCheckFailed"
 
 
 class FluxGitopsOperator(GitopsOperatorInterface):
@@ -24,13 +24,6 @@ def extract_commit_statuses(self, phase_data):
             phase_data['message'])
         kind = self._get_message_kind(phase_data)
 
-        # For Kustomization we have more detailed data to parse in addition to status.
-        if self._get_message_kind(phase_data) == KUSTOMIZATION_PHASE:
-            if phase_data['reason'] == PROGRESSING_SATE:
-                self._add_progression_summary(phase_data, commit_id, commit_statuses, kind)
-            elif phase_data['reason'] == HEALTH_CHECK_FAILED_SATE:
-                self._add_health_check_summary(phase_data, commit_id, commit_statuses, reason_message, kind)
-
         # Generic status message regardless of kind.
         status = self._new_git_commit_status(
             commit_id=commit_id,
@@ -40,6 +33,22 @@ def extract_commit_statuses(self, phase_data):
             kind=kind)
         commit_statuses.append(status)
 
+        # For Kustomization we have more detailed data to parse in addition to status.
+        if self._get_message_kind(phase_data) == KUSTOMIZATION_PHASE:
+            if phase_data['reason'] == PROGRESSING_STATE:
+                self._add_progression_summary(phase_data, commit_id, commit_statuses, kind)
+                # For Progressive state adding a generic message again so the overall Status will be "pending"
+                # (Bug in AzDO)
+                status = self._new_git_commit_status(
+                    commit_id=commit_id,
+                    status_name='Status',
+                    state=reason_state,
+                    message=reason_message,
+                    kind=kind)
+                commit_statuses.append(status)
+            elif phase_data['reason'] == HEALTH_CHECK_FAILED_STATE:
+                self._add_health_check_summary(phase_data, commit_id, commit_statuses, reason_message, kind)
+
         return commit_statuses
 
     def _add_progression_summary(self, phase_data, commit_id, commit_statuses, kind):
@@ -64,7 +73,7 @@ def _add_health_check_summary(self, phase_data, commit_id, commit_statuses, reas
                 status = self._new_git_commit_status(
                     commit_id=commit_id,
                     status_name=resource_name,
-                    state=HEALTH_CHECK_FAILED_SATE,
+                    state=HEALTH_CHECK_FAILED_STATE,
                     message=reason_message,
                     kind=kind)
                 commit_statuses.append(status)

diff --git a/src/orchestrators/azdo_cicd_orchestrator.py b/src/orchestrators/azdo_cicd_orchestrator.py
@@ -1,3 +1,6 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+
 import logging
 import requests
 from datetime import datetime, timedelta
@@ -11,9 +14,6 @@
 TASK_CUTOFF_DURATION = timedelta(minutes=MAX_TASK_TIMEOUT)
 
 
-# Copyright (c) Microsoft Corporation.
-# Licensed under the MIT License.
-
 class AzdoCicdOrchestrator(CicdOrchestratorInterface):
 
     def __init__(self, git_repository: GitRepositoryInterface):

diff --git a/src/repositories/azdo_git_repository.py b/src/repositories/azdo_git_repository.py
@@ -32,7 +32,7 @@ def post_commit_status(self, commit_status):
         data = {
             'state': azdo_status,
             'description': commit_status.status_name + ": " + commit_status.message,
-            'targetUrl': commit_status.callback_url + "/" + commit_status.commit_id,
+            'targetUrl': commit_status.callback_url + "?noop=" + commit_status.status_name,
             # Shows up as "genre/name" underneath the message and status.
             'context': {
                 'name': commit_status.status_name,