Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation of existing packages to sync #100

Merged
merged 2 commits into from Feb 18, 2020

Conversation

mdellweg
Copy link
Member

If force full mode is active, existing packages are validated and if needed
rescheduled for download.

closes #5170
https://pulp.plan.io/issues/5170

@mdellweg
Copy link
Member Author

Tests will follow.

@@ -59,6 +73,8 @@ def __init__(self, repo, conduit, config):
self.components = split_or_none(self.get_config().get('components'))
self.remove_missing = self.get_config().get_boolean(
constants.CONFIG_REMOVE_MISSING_UNITS, constants.CONFIG_REMOVE_MISSING_UNITS_DEFAULT)
self.validate = repo_controller.check_perform_full_sync(
self.repo.id, self.conduit, self.config)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized by looking at how the tests fail, that this is not the option i was looking for...

@mdellweg mdellweg force-pushed the verifying_full_sync branch 3 times, most recently from a9e6862 to 5bdaf21 Compare July 23, 2019 14:45
@@ -230,7 +230,7 @@ def test_SaveDownloadedUnits(self):

repo = self.repo.repo_obj
for path, unit in path_to_unit.items():
unit.save_and_associate.assert_called_once_with(path, repo)
unit.save_and_associate.assert_called_once_with(path, repo, force=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this default to False on its own?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but the function will actually be called with an explicit False in the new version.

@@ -59,6 +73,7 @@ def __init__(self, repo, conduit, config):
self.components = split_or_none(self.get_config().get('components'))
self.remove_missing = self.get_config().get_boolean(
constants.CONFIG_REMOVE_MISSING_UNITS, constants.CONFIG_REMOVE_MISSING_UNITS_DEFAULT)
self.validate = self.get_config().get_boolean(importer_constants.KEY_VALIDATE, False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly we are now deciding between verifying or not verifying based on an importer setting?
How does this compare to how RPM and others are handling advanced sync options?

@mdellweg
Copy link
Member Author

The test are failing due to pulp/pulp#3946

If the repair_sync option is true on sync, existing packages are validated
and, if needed, rescheduled for download.

closes #5170
https://pulp.plan.io/issues/5170
@m-bucher
Copy link
Contributor

Are we ready to merge this?
We also have a PR in katello waiting for this Katello/katello#8239

Test whether repair_sync triggers the redownload of units
@mdellweg
Copy link
Member Author

mdellweg commented Jan 2, 2020

I promised tests. Added a test to check the unit validation code.
I think this should be considered ready.

@mdellweg
Copy link
Member Author

@mibanescu as this is pulp2, may i ask you to have a look?

@mdellweg mdellweg added the pulp2 For issues/PRs relating to Pulp 2. label Feb 18, 2020
@mibanescu mibanescu merged commit 39ec4a9 into pulp:2-master Feb 18, 2020
@mdellweg mdellweg deleted the verifying_full_sync branch February 20, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pulp2 For issues/PRs relating to Pulp 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants