Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

introducing integrity #3465

Closed
wants to merge 1 commit into from
Closed

introducing integrity #3465

wants to merge 1 commit into from

Conversation

dparalen
Copy link
Contributor

@dparalen dparalen commented Apr 23, 2018

To test:

sudo -u apache pulp-integrity --check size --check broken_rpm_symlinks | jq '[.report[].repository] | unique'

TODO:

  • review/refactor the validator module to rely more on mixins than inheritance

@pep8speaks
Copy link

pep8speaks commented Apr 23, 2018

Hello @dparalen! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on June 08, 2018 at 08:34 Hours UTC

@pulpbot
Copy link
Member

pulpbot commented Apr 23, 2018

Can one of the admins verify this patch?

@dparalen dparalen force-pushed the integrity branch 3 times, most recently from 22731b7 to 02732ff Compare April 24, 2018 09:27
@dparalen dparalen changed the title [WIP] introducing integrity introducing integrity Apr 24, 2018
@dparalen dparalen force-pushed the integrity branch 2 times, most recently from 13260d1 to 1b816b9 Compare April 24, 2018 10:29
@asmacdo
Copy link
Contributor

asmacdo commented Apr 24, 2018

Is there an issue associated with this? Specifically, I'd like to see a summary of the motivation for this change.

@dparalen
Copy link
Contributor Author

@asmacdo https://pulp.plan.io/issues/2619 I forgotten a link in the commit message (just #ID there), will submit with next update

Copy link
Member

@dkliban dkliban left a comment

Choose a reason for hiding this comment

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

While running this on an older Pulp I ran into a problem where this tools was relying on a method in pulp_rpm that did not exist. I left a comment in the code to show where.

This tool should probably rely only on the model schema. Any other code could be significantly different from version to version of Pulp. Even though it is not DRY, you should probably just copy the methods you need from pulp_rpm into this tool. This way we'll be able to run this tool against older versions of Pulp.

Since the symlink validator is very much related to the yum_distributor, I recommend putting this tool in the pulp_rpm repository.

functools.partial(os.path.samefile, unit.storage_path) or
os.path.lexists
)
symlink_name = unit.get_symlink_name()
Copy link
Member

Choose a reason for hiding this comment

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

This method is not available in older versions of pulp_rpm.

yield validator.DarkPath(self, storage_path, DarkContentError(storage_path))


class BrokenSymlinksValidator(validator.MultiValidator):
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this validator is very specific to pulp_rpm.

@dparalen
Copy link
Contributor Author

@dkliban, thanks for the review!
moving the rpm tools to the pulp_rpm repo; see pulp/pulp_rpm#1104

@dparalen dparalen force-pushed the integrity branch 3 times, most recently from 69d7ae1 to 65effc4 Compare May 4, 2018 09:28
@dparalen dparalen force-pushed the integrity branch 5 times, most recently from 739653f to 75207f9 Compare May 23, 2018 13:41
@dparalen dparalen force-pushed the integrity branch 3 times, most recently from 75fa002 to 8d68fd5 Compare May 30, 2018 19:16
@dparalen dparalen changed the base branch from master to 2-master May 30, 2018 20:18
@dparalen dparalen force-pushed the integrity branch 3 times, most recently from f306bfa to e48a096 Compare May 30, 2018 21:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants