Skip to content
This repository has been archived by the owner on Oct 28, 2019. It is now read-only.

Commit

Permalink
Remove the GroupDownloader
Browse files Browse the repository at this point in the history
The GroupDownloader is not needed. See the ticket description for more
info.

Also the 'attach_url_to_exception' was only here to serve the
GroupDownloader so that is also being removed.

https://pulp.plan.io/issues/3923
closes #3923
  • Loading branch information
Brian Bouterse committed Aug 29, 2018
1 parent a175c11 commit d9eb035
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 317 deletions.
39 changes: 0 additions & 39 deletions docs/plugins/plugin-api/download.rst
Expand Up @@ -113,13 +113,6 @@ size or digest validation. There can also be protocol specific errors such as an
``aiohttp.ClientResponse`` being raised when a server responds with a 400+ response such as an HTTP
403.

If downloading synchronously, exceptions are raised from
:meth:`~pulpcore.plugin.download.BaseDownloader.fetch`. If downloading in parallel,
exceptions are raised when checking the `result()` method of a downloader. Exceptions encountered
while downloading is done by the :class:`~pulpcore.plugin.download.GroupDownloader` are
handled differently. See the :class:`~pulpcore.plugin.download.GroupDownloader` docs for
more information.

Any exception raised is a fatal exception and should likely be recorded with the
:meth:`~pulpcore.plugin.tasking.Task.append_non_fatal_error` interface. A fatal exception on a
single download likely does not cause an entire sync to fail, so a downloader's fatal exception is
Expand Down Expand Up @@ -156,36 +149,6 @@ To create a new protocol downloader implement a subclass of the
:class:`~pulpcore.plugin.download.BaseDownloader`. See the docs on
:class:`~pulpcore.plugin.download.BaseDownloader` for more information on the requirements.

.. _group-downloader:

Downloading Groups of Files
---------------------------

Motivation
##########

A content unit that requires multiple files to be all downloaded before the content unit can be
saved is a common problem. Consider a content unit `foo`, that requires three files, A, B, and C.
One option is to use the :ref:`DownloaderFactory <downloader-factory>` to generate a downloader for
each URL (A, B, C) and wait for those downloads to complete before saving the content unit `foo` and
its associated :class:`~pulpcore.plugin.models.Artifact` objects. The issue with this approach is
that you also want to download other content units, e.g. a unit named `bar` with files (D, E, and F)
and while waiting on A, B, and C you are not also downloading D, E, and F in parallel.

GroupDownloader Overview
########################

The GroupDownloader allows you to schedule a :class:`~pulpcore.plugin.download.Group`
of downloads in a way that results are returned when the entire Group is ready instead of
download-by-download. This is significant because multiple downloads from multiple groups still run
in parallel. See the examples below.

.. autoclass:: pulpcore.plugin.download.GroupDownloader
:members:

.. autoclass:: pulpcore.plugin.download.Group
:members:

.. _downloader-factory:

Download Factory
Expand Down Expand Up @@ -251,8 +214,6 @@ descendants of BaseDownloader.
.. autoclass:: pulpcore.plugin.download.BaseDownloader
:members:

.. autofunction:: pulpcore.plugin.download.attach_url_to_exception


.. _validation-exceptions:

Expand Down
3 changes: 1 addition & 2 deletions plugin/pulpcore/plugin/download/__init__.py
@@ -1,7 +1,6 @@
from .base import attach_url_to_exception, BaseDownloader, DownloadResult # noqa
from .base import BaseDownloader, DownloadResult # noqa
from .exceptions import (DigestValidationError, DownloaderValidationError, # noqa
SizeValidationError) # noqa
from .factory import DownloaderFactory # noqa
from .file import FileDownloader # noqa
from .http import HttpDownloader # noqa
from .group import Group, GroupDownloader # noqa
39 changes: 1 addition & 38 deletions plugin/pulpcore/plugin/download/base.py
Expand Up @@ -12,49 +12,17 @@
log = logging.getLogger(__name__)


DownloadResult = namedtuple('DownloadResult', ['url', 'artifact_attributes', 'path', 'exception'])
DownloadResult = namedtuple('DownloadResult', ['url', 'artifact_attributes', 'path'])
"""
Args:
url (str): The url corresponding with the download.
path (str): The absolute path to the saved file
artifact_attributes (dict): Contains keys corresponding with
:class:`~pulpcore.plugin.models.Artifact` fields. This includes the computed digest values
along with size information.
exception (Exception): Any downloader exception emitted while the
:class:`~pulpcore.plugin.download.GroupDownloader` is downloading. Otherwise this
value is `None`.
"""


def attach_url_to_exception(func):
"""
A decorator that attaches the `url` to any exception emitted by a downloader's `run()` method.
>>> class MyCustomDownloader(BaseDownloader)
>>> @attach_url_to_exception
>>> async def run(self):
>>> pass # downloader implementation of run() goes here
The url is stored on the exception as the `_pulp_url` attribute. This is used by the
:class:`~pulpcore.plugin.download.GroupDownloader` to know the url a given exception
is for.
Args:
func: The method being decorated. This is expected to be the `run()` method of a subclass of
:class:`~pulpcore.plugin.download.BaseDownloader`
Returns:
A coroutine that will attach the `url` to any exception emitted by `func`
"""
async def wrapper(downloader):
try:
return await func(downloader)
except Exception as error:
error._pulp_url = downloader.url
raise error
return wrapper


class BaseDownloader:
"""
The base class of all downloaders, providing digest calculation, validation, and file handling.
Expand Down Expand Up @@ -235,11 +203,6 @@ async def run(self):
:class:`~pulpcore.plugin.download.DownloadResult` is usually set to the
:attr:`~pulpcore.plugin.download.BaseDownloader.artifact_attributes` property value.
It is also expected that the subclass implementation be decorated with the
:class:`~pulpcore.plugin.download.attach_url_to_exception` decorator. This is
required to allow the :class:`~pulpcore.plugin.download.GroupDownloader` to properly
record the exceptions emitted from subclassed downloaders.
Returns:
:class:`~pulpcore.plugin.download.DownloadResult`
Expand Down
5 changes: 2 additions & 3 deletions plugin/pulpcore/plugin/download/file.py
Expand Up @@ -4,7 +4,7 @@

import aiofiles

from .base import attach_url_to_exception, BaseDownloader, DownloadResult
from .base import BaseDownloader, DownloadResult


class FileDownloader(BaseDownloader):
Expand Down Expand Up @@ -32,7 +32,6 @@ def __init__(self, url, **kwargs):
self._path = os.path.abspath(os.path.join(p.netloc, p.path))
super().__init__(url, **kwargs)

@attach_url_to_exception
async def run(self):
"""
Read, validate, and compute digests on the `url`. This is a coroutine.
Expand All @@ -48,4 +47,4 @@ async def run(self):
break # the reading is done
self.handle_data(chunk)
return DownloadResult(path=self._path, artifact_attributes=self.artifact_attributes,
url=self.url, exception=None)
url=self.url)
232 changes: 0 additions & 232 deletions plugin/pulpcore/plugin/download/group.py

This file was deleted.

0 comments on commit d9eb035

Please sign in to comment.