Skip to content

Commit

Permalink
Allow developers to override the GetLocalUnitsStep's available_units.
Browse files Browse the repository at this point in the history
Some types are a mixture of Units and Metadata, and need to be treated separately. The GetLocalUnitsStep did
not allow developers to specify what data structure would contain the iterable of available Units. This
commit adds the ability for developers to give a pointer to that data structure while maintaining the
previous behavior if it is not specified for backwards compatibility.

This is needed for the Docker plugin Mongoengine conversion as it reduces the number of steps that are needed
in its importer's code.

https://pulp.plan.io/issues/863

re #863
  • Loading branch information
Randy Barlow committed Nov 30, 2015
1 parent 28c3cf5 commit 89b8935
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 11 deletions.
25 changes: 17 additions & 8 deletions server/pulp/plugins/util/publish_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -1169,20 +1169,24 @@ def finalize(self):

class GetLocalUnitsStep(SaveUnitsStep):
"""
Associate existing units & produce a list of unknown units
Associate existing units & produce a list of unknown units.
Given an iterator of units, associate the ones that are already in Pulp with the
repository and create a list of all the units that do not yet exist in Pulp.
This requires a parent step with the attribute "available_units",
which must be an iterable of unit model instances with the unit keys populated
This requires an iterable of "available_units", which must be an iterable of unit model
instances with the unit keys populated. By default it will use it's parent step's
available_units attribute, but can be overridden in the constructor.
"""

def __init__(self, importer_type, unit_pagination_size=50, **kwargs):
def __init__(self, importer_type, unit_pagination_size=50, available_units=None, **kwargs):
"""
:param importer_type: unique identifier for the type of importer
:type importer_type: basestring
:param importer_type: unique identifier for the type of importer
:type importer_type: basestring
:param unit_pagination_size: How many units should be queried at one time (default 50)
:type importer_type: int
:type importer_type: int
:param available_units: An iterable of Units available for retrieval. This defaults to
this step's parent's available_units attribute if not provided.
:type available_units: iterable
"""
super(GetLocalUnitsStep, self).__init__(step_type=reporting_constants.SYNC_STEP_GET_LOCAL,
plugin_type=importer_type,
Expand All @@ -1192,6 +1196,7 @@ def __init__(self, importer_type, unit_pagination_size=50, **kwargs):
# list of unit model instances
self.units_to_download = []
self.unit_pagination_size = unit_pagination_size
self.available_units = available_units

def process_main(self, item=None):
"""
Expand All @@ -1204,7 +1209,11 @@ def process_main(self, item=None):
# any units that are already in pulp
units_we_already_had = set()

for units_group in misc.paginate(self.parent.available_units, self.unit_pagination_size):
# If available_units was defined in the constructor, let's use it. Otherwise let's use the
# default of self.parent.available_units
available_units = self.available_units or self.parent.available_units

for units_group in misc.paginate(available_units, self.unit_pagination_size):
# Get this group of units
query = units_controller.find_units(units_group)

Expand Down
55 changes: 52 additions & 3 deletions server/test/unit/plugins/util/test_publish_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,27 @@ def setUp(self):
self.step.conduit = MagicMock()
self.parent.available_units = []

def test___init___default_available_units(self, find_units, associate):
"""
Assert that the __init__() method correctly defaults to a value of None for the
available_units parameter.
"""
step = publish_step.GetLocalUnitsStep('fake_importer_type', repo='fake_repo')

self.assertEqual(step.available_units, None)

def test___init___with_available_units(self, find_units, associate):
"""
Assert that the __init__() method allows the user to override the value of None for the
available_units parameter.
"""
available_units = ['unit1', 'unit2']

step = publish_step.GetLocalUnitsStep('fake_importer_type', repo='fake_repo',
available_units=available_units)

self.assertEqual(step.available_units, available_units)

def test_no_available_units(self, mock_find_units, mock_associate):
self.step.process_main()

Expand Down Expand Up @@ -1077,7 +1098,8 @@ def test_saves_unit(self, mock_find_units, mock_associate):
def test_populates_units_to_download(self, mock_find_units, mock_associate):
"""
Test that if a unit does not exist in the database it is added to the
units_to_download list
units_to_download list. This test also tests the case when the step is not constructed with
an available_units parameter that the parent step's available units is used.
"""
demo_1 = self.DemoModel(key_field='a')
demo_2 = self.DemoModel(key_field='b')
Expand All @@ -1088,11 +1110,38 @@ def test_populates_units_to_download(self, mock_find_units, mock_associate):
self.step.process_main()
mock_find_units.assert_called_once_with((demo_1, demo_2))

# The one that exists is associated
# the one that exists is associated
mock_associate.assert_called_once_with('fake_repo', existing_demo)
# The one that does not exist yet is added to the download list
# the one that does not exist yet is added to the download list
self.assertEqual(self.step.units_to_download, [demo_1])

def test_uses_passed_available_units_when_requested(self, mock_find_units, mock_associate):
"""
Assert that if the step is constructed with the default available_units, the step's parent's
available_units attribute is used.
"""
demo_1 = self.DemoModel(key_field='a')
demo_2 = self.DemoModel(key_field='b')
demo_3 = self.DemoModel(key_field='c')
self.parent.available_units = [demo_1, demo_2]
available_units = [demo_1, demo_2, demo_3]
step = publish_step.GetLocalUnitsStep('fake_importer_type', repo='fake_repo',
available_units=available_units)
step.parent = self.parent
step.conduit = MagicMock()
existing_demo = self.DemoModel(key_field='b', id='foo')
mock_find_units.return_value = [existing_demo]

step.process_main()

# The parent step's available units only has demo 1 and 2, so this asserts that that is
# being ignored and the correct available_units is being used instead.
mock_find_units.assert_called_once_with((demo_1, demo_2, demo_3))
# the one that exists is associated
mock_associate.assert_called_once_with('fake_repo', existing_demo)
# the two that do not exist yet are added to the download list
self.assertEqual(step.units_to_download, [demo_1, demo_3])


class TestSaveUnitsStep(unittest.TestCase):

Expand Down

0 comments on commit 89b8935

Please sign in to comment.