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.

Sign up for GitHub

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

Explicit cleanup for instances #1090

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+11 −0
Open

Explicit cleanup for instances #1090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0906255
pass cleanupFullPaths on to farm publish
sjt-rvx Jan 16, 2025
4e95941
have explicit cleanup also check for keys on instances, not just the …
sjt-rvx Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions client/ayon_core/pipeline/farm/pyblish_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ def create_skeleton_instance(
persistent = instance.data.get("stagingDir_persistent") is True
instance_skeleton_data["stagingDir_persistent"] = persistent

explicit_cleanup_paths = instance.data.get("cleanupFullPaths", [])
if len(explicit_cleanup_paths) > 0:
instance_skeleton_data["cleanupFullPaths"] = explicit_cleanup_paths

return instance_skeleton_data


Expand Down
7 changes: 7 additions & 0 deletions client/ayon_core/plugins/publish/cleanup_explicit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ def process(self, context):
self._remove_full_paths(cleanup_full_paths)
self._remove_empty_dirs(cleanup_empty_dirs)

for instance in context:
instance_cleanup_full_paths = instance.data.get("cleanupFullPaths")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this directly clean it up - even BEFORE it hits the farm - in the local publish session? (likely less of a problem because it didn't create data yet) but I wonder if that's a potential unintentional side effect of this?

Because now this is in the instance.data now and also in the instance data when the publish job runs on the farm. My gut feeling says that you're trying to say: "when the farm job finished, then remove these files please" - but now it also deletes them now (if they happened to already be there)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely right. That is indeed the side-effect and it could bite you hard .

And you are correct in my desired effect. I re-read cleanup_farm.py and it does exactly what I need. I thought I had read through it the other day and it did something wildly different then 😁 .

So this is wholly unnecessary. I'll probably make another merge request to add 'gaffer' to the allowed hosts in that plugin. This PR can be deleted.

instance_cleanup_empty_dirs = instance.data.get("cleanupEmptyDirs")

self._remove_full_paths(instance_cleanup_full_paths)
self._remove_empty_dirs(instance_cleanup_empty_dirs)

def _remove_full_paths(self, full_paths):
"""Remove files and folders from disc.

Expand Down
Loading