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 touch to QuerySets of artifacts or content #1537

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

mdellweg
Copy link
Member

fixes TBA

This was just a random thought i had when writing these lines: https://github.com/pulp/pulp_container/blob/d25b01d4fb8aed1b264a9dcfc91461fe92d630b8/pulp_container/app/viewsets.py#L748

Not sure if it really helps there, also it's really kind of late for 3.14. If you like this i can do the issue dance for it.

@mdellweg mdellweg marked this pull request as draft July 30, 2021 16:30
@daviddavis
Copy link
Contributor

Ha! I actually had the same idea while reviewing these plugins PRs.

@pulp pulp deleted a comment from pulpbot Aug 12, 2021
@mdellweg mdellweg marked this pull request as ready for review August 12, 2021 09:13
@@ -470,7 +482,7 @@ class Content(MasterModel, QueryMixin):
_artifacts = models.ManyToManyField(Artifact, through="ContentArtifact")
timestamp_of_interest = models.DateTimeField(auto_now=True)

objects = ContentManager()
objects = ContentManager.from_queryset(BulkTouchQuerySet)()
Copy link
Member Author

Choose a reason for hiding this comment

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

If it wasn't for the content_pks parameter, i think we could move orphaned to the query set as well.
But it would change an interface now.

@pulpbot
Copy link
Member

pulpbot commented Aug 12, 2021

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

@mdellweg mdellweg requested a review from dkliban August 16, 2021 08:55
Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

A unit test would be nice but not required.

@mdellweg
Copy link
Member Author

We believe this will get test coverage, once it's been used by #1553 .

@mdellweg mdellweg merged commit aafe376 into pulp:master Aug 16, 2021
@mdellweg mdellweg deleted the bulk_touch branch August 16, 2021 14:05
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.

None yet

4 participants