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

Fix error when listing repo version while running orphan cleanup #1681

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Oct 11, 2021

fixes: #9481

Since orphan cleanup became a non-blocking task it was possible for this query to "return'" None if the first Content was deleted before the cast call was completed. Now it should use a Content object that is apart of RepositoryVersion.

Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html

@pulpbot
Copy link
Member

pulpbot commented Oct 11, 2021

Attached issue: https://pulp.plan.io/issues/9481

@gerrod3 gerrod3 force-pushed the repo_ver_content_href branch 2 times, most recently from f989a28 to 5b03531 Compare October 11, 2021 21:39
ctype_query = Content.objects.filter(
pulp_type=self.content_type, pk__in=repository_content.values_list("content", flat=True)
)
ctype_model = ctype_query.first().cast().__class__
Copy link
Contributor

@dralley dralley Oct 19, 2021

Choose a reason for hiding this comment

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

The docstring of this function doesn't match what it is doing (or what it was doing previously, to be fair). Could you update it to convey the requirement that it be capable of handling the types of content "removed" from the repository version as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"present" is the misleading term

For each content type present in the RepositoryVersion, create the URL of the viewset of
that variety of content along with a query parameter which filters it by presence in this
RepositoryVersion.
For each content type that is or was apart of the RepositoryVersion, create the URL of the
Copy link
Contributor

@dralley dralley Oct 20, 2021

Choose a reason for hiding this comment

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

that is present in or removed from this RepositoryVersion

Copy link
Contributor

@dralley dralley left a comment

Choose a reason for hiding this comment

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

Suggested a slightly different phrasing, LGTM otherwise.

@dralley dralley merged commit 62670dc into pulp:master Oct 20, 2021
@gerrod3 gerrod3 deleted the repo_ver_content_href branch October 20, 2021 17:17
@rossengeorgiev
Copy link

Just tested this patch by manually making the changes on katello 4.1, and I still get the error. Details: https://community.theforeman.org/t/pulpdebclient-apierror-http-500-when-running-katello-delete-orphaned-content/25468/9?u=rgp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants