Touch artifacts and content to prevent cleanup #362
Conversation
|
Attached issue: https://pulp.plan.io/issues/9134 |
pulp_container/app/serializers.py
Outdated
| # TODO Evaluate, whether we need touch here. | ||
| # The manifest is in a repository version, so not orphaned, | ||
| # but can the version be deleted between this and using it? | ||
| manifest.touch() |
There was a problem hiding this comment.
there is a small window for such case, so i probably would keep it.
| @@ -668,7 +669,7 @@ def copy_tags(self, request, pk): | |||
|
|
|||
| result = dispatch( | |||
| tasks.recursive_add_content, | |||
| [repository], | |||
| [repository, source_latest.repository], | |||
There was a problem hiding this comment.
I think, this was actually a bug.
| @@ -705,7 +706,7 @@ def copy_manifests(self, request, pk): | |||
| manifests_to_add = manifests_in_repo.filter(**filters) | |||
| result = dispatch( | |||
| tasks.recursive_add_content, | |||
| [repository], | |||
| [repository, source_latest.repository], | |||
pulp_container/app/tasks/builder.py
Outdated
| @@ -111,12 +112,14 @@ def build_image_from_containerfile( | |||
|
|
|||
| """ | |||
| containerfile = Artifact.objects.get(pk=containerfile_pk) | |||
| containerfile.touch() | |||
There was a problem hiding this comment.
i think, here, the line is not needed as well since we touched it in the viewset
There was a problem hiding this comment.
I had the same thougt. But then i decided to go ahead anyway, because there could be a considerate a amount of time between the request and the task, and it would be just so inconvenient if the artifact vanished between here and being used after reading all the other artifacts from cloud storage.
I'm open for discussion though.
There was a problem hiding this comment.
with this logic then we should double that use of touch in viewset and inside of the task..
the default is set to 1day( for the orphan cleanup), i'd still insist on not doubling this.
| @@ -107,6 +107,7 @@ async def run(self): | |||
| except IntegrityError: | |||
| del tag.artifact_attributes["file"] | |||
| saved_artifact = Artifact.objects.get(**tag.artifact_attributes) | |||
pulp_container/app/tasks/builder.py
Outdated
| repository = ContainerRepository.objects.get(pk=repository_pk) | ||
| name = str(uuid4()) | ||
| with tempfile.TemporaryDirectory(".") as working_directory: | ||
| path = "{}/".format(working_directory.path) | ||
| for key, val in artifacts.items(): | ||
| artifact = Artifact.objects.get(pk=key) | ||
| artifact.touch() |
There was a problem hiding this comment.
thank you for this pr, it looks good to me - i would rather move touch outside of the task into the viewset #362 (comment) and this one seems to be redundant #362 (comment)
This change prepares for the async orphan cleanup that is expected to come with pulpcore 3.15. fixes #9134
This change prepares for the async orphan cleanup that is expected to
come with pulpcore 3.15.
fixes #9134