Skip to content

Commit

Permalink
1131062 - propogate cancelation even when download exception occurs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
beav committed Aug 27, 2014
1 parent 0b5276a commit c8c4b4e
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions pulp_puppet_common/pulp_puppet/common/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
STATE_SUCCESS = 'success'
STATE_FAILED = 'failed'
STATE_SKIPPED = 'skipped'
STATE_CANCELED = 'canceled'

COMPLETE_STATES = (STATE_SUCCESS, STATE_FAILED, STATE_SKIPPED)

Expand Down
5 changes: 4 additions & 1 deletion pulp_puppet_common/pulp_puppet/common/sync_progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 6 additions & 1 deletion pulp_puppet_plugins/pulp_puppet/plugins/importers/forge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
21 changes: 21 additions & 0 deletions pulp_puppet_plugins/test/unit/plugins/importer/test_forge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c8c4b4e

Please sign in to comment.