From c8c4b4ed4bc622893c12d20e23b6e2c56f9d23de Mon Sep 17 00:00:00 2001
From: Chris Duryee <cduryee@redhat.com>
Date: Wed, 27 Aug 2014 19:00:33 -0400
Subject: [PATCH] 1131062 - propogate cancelation even when download exception
 occurs

Previously, if a metadata download task was canceled, nectar would still call
back after the download was finished and an exception would be raised since
SynchronizeWithPuppetForge would not be set up to accept the cancelation. This
would cause an exception, which would mark the already-canceled task as failed.
Exceptions related to downloading would triggered this cancel->failed behavior
as well.

The new behavior is to check if a download was canceled before raising an
exception. In this case, we mark the metadata download state as STATE_CANCELED
and continue. Later during report generation when the metadata and module
download states are coalesed into a single report, we set the canceled flag on
the report.  This lets platform handle the cancelation correctly.
---
 .../pulp_puppet/common/constants.py           |  1 +
 .../pulp_puppet/common/sync_progress.py       |  5 ++++-
 .../pulp_puppet/plugins/importers/forge.py    |  7 ++++++-
 .../test/unit/plugins/importer/test_forge.py  | 21 +++++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/pulp_puppet_common/pulp_puppet/common/constants.py b/pulp_puppet_common/pulp_puppet/common/constants.py
index 6013059a..036ea4dc 100644
--- a/pulp_puppet_common/pulp_puppet/common/constants.py
+++ b/pulp_puppet_common/pulp_puppet/common/constants.py
@@ -87,6 +87,7 @@
 STATE_SUCCESS = 'success'
 STATE_FAILED = 'failed'
 STATE_SKIPPED = 'skipped'
+STATE_CANCELED = 'canceled'
 
 COMPLETE_STATES = (STATE_SUCCESS, STATE_FAILED, STATE_SKIPPED)
 
diff --git a/pulp_puppet_common/pulp_puppet/common/sync_progress.py b/pulp_puppet_common/pulp_puppet/common/sync_progress.py
index bd05a11d..d7fc5d20 100644
--- a/pulp_puppet_common/pulp_puppet/common/sync_progress.py
+++ b/pulp_puppet_common/pulp_puppet/common/sync_progress.py
@@ -17,7 +17,7 @@
 """
 
 from pulp_puppet.common import reporting
-from pulp_puppet.common.constants import STATE_NOT_STARTED, STATE_SUCCESS
+from pulp_puppet.common.constants import STATE_NOT_STARTED, STATE_SUCCESS, STATE_CANCELED
 
 
 class SyncProgressReport(object):
@@ -137,6 +137,9 @@ def build_final_report(self):
         else:
             report = self.conduit.build_failure_report(summary, details)
 
+        if self.metadata_state == STATE_CANCELED:
+            report.canceled_flag = True
+
         return report
 
     def build_progress_report(self):
diff --git a/pulp_puppet_plugins/pulp_puppet/plugins/importers/forge.py b/pulp_puppet_plugins/pulp_puppet/plugins/importers/forge.py
index 1ea5d4e6..82282ba3 100644
--- a/pulp_puppet_plugins/pulp_puppet/plugins/importers/forge.py
+++ b/pulp_puppet_plugins/pulp_puppet/plugins/importers/forge.py
@@ -22,7 +22,8 @@
 from pulp.plugins.conduits.mixins import UnitAssociationCriteria
 
 from pulp_puppet.common import constants
-from pulp_puppet.common.constants import (STATE_FAILED, STATE_RUNNING, STATE_SUCCESS)
+from pulp_puppet.common.constants import (STATE_FAILED, STATE_RUNNING,\
+                                          STATE_SUCCESS, STATE_CANCELED)
 from pulp_puppet.common.model import RepositoryMetadata, Module
 from pulp_puppet.common.sync_progress import SyncProgressReport
 from pulp_puppet.plugins.importers import metadata
@@ -123,6 +124,10 @@ def _parse_metadata(self):
             metadata_json_docs = downloader.retrieve_metadata(self.progress_report)
 
         except Exception, e:
+            if self._canceled:
+                logger.warn('Exception occurred on canceled metadata download: %s' % e)
+                self.progress_report.metadata_state = STATE_CANCELED
+                return None
             logger.exception('Exception while retrieving metadata for repository <%s>' % self.repo.id)
             self.progress_report.metadata_state = STATE_FAILED
             self.progress_report.metadata_error_message = _('Error downloading metadata')
diff --git a/pulp_puppet_plugins/test/unit/plugins/importer/test_forge.py b/pulp_puppet_plugins/test/unit/plugins/importer/test_forge.py
index 32a30038..e815858d 100644
--- a/pulp_puppet_plugins/test/unit/plugins/importer/test_forge.py
+++ b/pulp_puppet_plugins/test/unit/plugins/importer/test_forge.py
@@ -274,6 +274,27 @@ def test_parse_metadata_retrieve_exception(self, mock_create):
 
         self.assertEqual(pr.modules_state, constants.STATE_NOT_STARTED)
 
+    @mock.patch('pulp_puppet.plugins.importers.forge.SynchronizeWithPuppetForge._create_downloader')
+    def test_parse_metadata_retrieve_exception_canceled(self, mock_create):
+        # Setup
+        swpf = SynchronizeWithPuppetForge(self.repo, self.conduit, self.config)
+
+        def _side_effect(*args, **kwargs):
+            swpf.cancel()
+            raise Exception("some download error")
+
+        mock_create.side_effect = _side_effect
+
+        # Test
+        report = swpf().build_final_report()
+
+        # Verify
+        self.assertTrue(report.canceled_flag)
+
+        pr = swpf.progress_report
+        self.assertEqual(pr.metadata_state, constants.STATE_CANCELED)
+        self.assertEqual(pr.modules_state, constants.STATE_NOT_STARTED)
+
     @mock.patch('pulp_puppet.plugins.importers.downloaders.local.LocalDownloader.retrieve_metadata')
     def test_parse_metadata_parse_exception(self, mock_retrieve):
         # Setup