MAAS

Merge lp://staging/~julian-edwards/maas/download-service-exceptions into lp://staging/~maas-committers/maas/trunk

download-service-exceptions
Merge into trunk

Proposed by Julian Edwards on 2014-08-27

Status:

Merged

Approved by:

Julian Edwards on 2014-08-27

Approved revision:

no longer in the source branch.

Merged at revision:

2828

Proposed branch:

lp://staging/~julian-edwards/maas/download-service-exceptions

Merge into:

lp://staging/~maas-committers/maas/trunk

Diff against target:

121 lines (+50/-6)

2 files modified

src/provisioningserver/pserv_services/image_download_service.py (+22/-5)
src/provisioningserver/pserv_services/tests/test_image_download_service.py (+28/-1)

To merge this branch:

bzr merge lp://staging/~julian-edwards/maas/download-service-exceptions

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jeroen T. Vermeulen (community)		2014-08-27	Approve on 2014-08-27
Review via email: mp+232352@code.staging.launchpad.net

Commit message

Ensure that exceptions are caught inside the PeriodicImageDownloadService, which would otherwise make it stop running.

Description of the change

Revision history for this message

Jeroen T. Vermeulen (jtv) wrote on 2014-08-27:

Nicely done. Just one note: shouldn't the existing tests then call try_download instead of maybe_start_download, so that the success case is covered?

review: Approve

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-27:

On Wednesday 27 August 2014 07:15:51 you wrote:
> Review: Approve
>
> Nicely done. Just one note: shouldn't the existing tests then call
> try_download instead of maybe_start_download, so that the success case is
> covered?

It's already doing that; it relies on the service itself calling the functions
in the service object.

Thanks for the review!