Repository: SFDO-Tooling/CumulusCI
Pull request #3394 (2022-10-13 15:23:52)

Improve handling of unknown statuses in mc deploy task
## Issues Fixed

* Fixed an issue where unknown deploy statuses could cause polling to hang in the `deploy_marketing_cloud_package` task. 


Patch:
diff --git a/cumulusci/tasks/marketing_cloud/deploy.py b/cumulusci/tasks/marketing_cloud/deploy.py
index e69ac05fa..31117a6a2 100644
--- a/cumulusci/tasks/marketing_cloud/deploy.py
+++ b/cumulusci/tasks/marketing_cloud/deploy.py
@@ -24,8 +24,11 @@ PAYLOAD_NAMESPACE_VALUES = {
     "timestamp": True,
 }
 
-DEPLOY_FINISHED_STATUS = "DONE"
-DEPLOY_ERROR_STATUS = "FATAL_ERROR"
+IN_PROGRESS_STATUS = "IN_PROGRESS"
+FINISHED_STATUSES = ("DONE",)
+ERROR_STATUSES = ("FATAL_ERROR", "ERROR")
+
+UNKNOWN_STATUS_MESSAGE = "Received unknown deploy status: {}"
 
 
 class MarketingCloudDeployTask(BaseMarketingCloudTask):
@@ -84,10 +87,10 @@ class MarketingCloudDeployTask(BaseMarketingCloudTask):
             json=payload,
             headers=self.headers,
         )
-        result = safe_json_from_response(response)
+        response_data = safe_json_from_response(response)
 
-        self.job_id = result["id"]
-        self.logger.info(f"Started job {self.job_id}")
+        self.job_id = response_data["id"]
+        self.logger.info(f"Started deploy job with Id: {self.job_id}")
         self._poll()
 
     def _poll_action(self):
@@ -98,14 +101,29 @@ class MarketingCloudDeployTask(BaseMarketingCloudTask):
         response = requests.get(
             f"{self.endpoint}/deployments/{self.job_id}", headers=self.headers
         )
-        result = safe_json_from_response(response)
-        self.logger.info(f"Waiting [{result['status']}]...")
-        if result["status"] == DEPLOY_FINISHED_STATUS:
+        response_data = safe_json_from_response(response)
+        deploy_status = response_data["status"]
+        self.logger.info(f"Deployment status is: {deploy_status}")
+
+        if deploy_status != IN_PROGRESS_STATUS:
+            self._process_completed_deploy(response_data)
+
+    def _process_completed_deploy(self, response_data: dict):
+        deploy_status = response_data["status"]
+        assert (
+            deploy_status != IN_PROGRESS_STATUS
+        ), "Deploy should be in a completed state before processing."
+
+        if deploy_status in FINISHED_STATUSES:
+            self.poll_complete = True
+            self._validate_response(response_data)
+        elif deploy_status in ERROR_STATUSES:
             self.poll_complete = True
-            self._validate_response(result)
-        elif result["status"] == DEPLOY_ERROR_STATUS:
+            self._report_error(response_data)
+        else:
+            self.logger.error(UNKNOWN_STATUS_MESSAGE.format(deploy_status))
             self.poll_complete = True
-            self._report_fatal_error(result)
+            self._report_error(response_data)
 
     def _construct_payload(self, dir_path, custom_inputs=None):
         dir_path = Path(dir_path)
@@ -171,10 +189,11 @@ class MarketingCloudDeployTask(BaseMarketingCloudTask):
 
         self.logger.info("Deployment completed successfully.")
 
-    def _report_fatal_error(self, result: dict):
+    def _report_error(self, response_data: dict):
+        deploy_status = response_data["status"]
         self.logger.error(
-            f"> {DEPLOY_ERROR_STATUS} received. Dumping response from Marketing Cloud:\n{result}"
+            f"Received status of: {deploy_status}. Received the following data from Marketing Cloud:\n{response_data}^"
         )
         raise DeploymentException(
-            f"Marketing Cloud deploy finished with status of: {DEPLOY_ERROR_STATUS}"
+            f"Marketing Cloud deploy finished with status of: {deploy_status}"
         )


Test patch:
diff --git a/cumulusci/tasks/marketing_cloud/tests/test_deploy.py b/cumulusci/tasks/marketing_cloud/tests/test_deploy.py
index fb6c2cbb4..b595b244c 100644
--- a/cumulusci/tasks/marketing_cloud/tests/test_deploy.py
+++ b/cumulusci/tasks/marketing_cloud/tests/test_deploy.py
@@ -14,6 +14,7 @@ from cumulusci.core.config.marketing_cloud_service_config import (
 from cumulusci.core.exceptions import DeploymentException
 from cumulusci.tasks.marketing_cloud.deploy import (
     MCPM_ENDPOINT,
+    UNKNOWN_STATUS_MESSAGE,
     MarketingCloudDeployTask,
 )
 from cumulusci.tasks.marketing_cloud.mc_constants import MC_API_VERSION
@@ -175,6 +176,28 @@ class TestMarketingCloudDeployTask:
         with pytest.raises(DeploymentException):
             task._run_task()
 
+    def test_run_task__unknown_deploy_status(self, task, mocked_responses, caplog):
+        unknown_status = "FOOBAR"
+        mocked_responses.add(
+            "POST",
+            f"{MCPM_ENDPOINT.format(STACK_KEY)}/deployments",
+            json={"id": "JOBID"},
+        )
+        mocked_responses.add(
+            "GET",
+            f"{MCPM_ENDPOINT.format(STACK_KEY)}/deployments/JOBID",
+            json={
+                "status": unknown_status,
+                "entities": {},
+            },
+        )
+        caplog.clear()
+        with pytest.raises(DeploymentException):
+            task._run_task()
+
+        logged_messages = [log.message for log in caplog.records]
+        assert UNKNOWN_STATUS_MESSAGE.format(unknown_status) in logged_messages
+
     def test_zipfile_not_valid(self, task):
         task.options["package_zip_file"] = "not-a-valid-file.zip"
         task.logger = mock.Mock()