-
Notifications
You must be signed in to change notification settings - Fork 43
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
Support integrating a folder as files for a representation #1113
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I thought the plan was to zip the directory. Is it happening somewhere else ?
The discussion was whether that'd be good or not. See the issue description here: ynput/ayon-silhouette#7 (comment) Zipping it could work for publishing, etc. but we also allow e.g. "rendering from the published file" which default deadline render plugins for e.g. Silhouette wouldn't support if for example the file was a That issue also exists with Harmony - we need a custom deadline rendering plug-in for Toonboon Harmony just to support rendering from Ideas are welcome - exactly why I've tried to raise this topic a few times. |
NOTE: Representation paths must contain paths to files, not folders, otherwise you'll break site-sync. |
Yup - as mentioned in the PR description I didn't test Site Sync myself but I did assume that would be the case (and I had checked with @kalisp about that too). Anyway, this is at a point where we should make the decision what way outweighs the pros and cons:
My biggest issue with point 2) is deadline rendering from a published workfile, in essence it'd mean that we'd need to customize the deadline render plug-in for the host application so that it can render from a Tagging @antirotor @mkolar @philippe-ynput to make the decision on whether this PR is "ok" (with a potential site-sync follow up fix to support folders too) or a hard pass (deciding to never allow a directory to be a representation's "files"). |
What about using Deadline events for unzipping it just before the render. Like onJobSubmit or similar. This would allow us to use vanilla Deadline plug-in. I am not saying it is perfect solution but it is probably the easiest one as you don't need to rewrite integrator, site sync or Deadline plug-in. Conceptually I don't mind having folder as a representation even though I would be very very careful about how and where it is used as you'll loose control (but that aspect is the same for zip). Performance wise it really depends on the scenario, server, number of files in the folder and their size. So there is no clear winner. In the representation traits I wasn't counting with them explicitly, but there is the Location trait and the Bundle trait that can be used to describe a folder. So to wrap this rant, I would go with zips now and then open the wider discussion about it. |
A few thoughts:
|
With zips there is one caveat though and that's that they will not retain user permissions and ownership (unless you tar before). I don't think it poses some issues as we try to stay away from file permissions but for pipeline tool in various studio deployments it needs to be considered. |
I agree that zip would be better overall and would allow to keep most other areas of the pipeline intact. The introducing extra logic to unpack and pack during open and publish doesn't seem that bad to be honest. |
But actually. It might very well be the best to leave folders as they are (with extension though) in the work area and workfiles app and only zip for publishing. That might be the best of both worlds. |
End goal is that we must have file paths in representation In theory integrator could just copy paste the content from source folder to destination folder, and store the file paths inside the destination folder to representation entity (similar to how resources are handled now). Question is how would load plugin know how to process such representation... Also there is no extension on the representation entity, which might be issue too. |
Just wanted to remark that the implementation here worked fine for my use case - and if the site sync would allow transfering "folders" and hash them accordingly (without the need to e.g. track the individual files inside it in the database) then it technically still works ok? The behavior would then be similar to how we're not tracking what the contents are of a |
The only downside here is Farm rendering - usually a farm render setup, like e.g. deadline just renders a filepath. We can't pass it the
Either way, if we do unzip on the farm - where do unzip to? Because one might get into the situation where e.g. many machines may try to unzip at the same time. So I guess the unzipped project then goes into |
It can't support folders transfer. Directory, on any disk-like integration (disk, gdrive, any cloud), is just metadata of file/s, and does not have any information for transfer if you don't have access to the folder (which is the point of site sync), we need to know about files, their sizes, their hashes etc. So representation paths MUST contain only paths to files, NOT folders. Like I've said, integrator should add all subfiles of given folder to representation paths, if it finds out it is a folder. |
If doing that - would it still be fine if the result from |
The loader that would be loading the representation should know that he can't use Paths on representation is not to mimic what template and context do, but to collect files that are really related to the representaion. |
In that case it still wouldn't solve the published workfiles, because the Workfiles tool > Copy & Open logic wouldn't be able to differentiate on that. |
Would it be fair to say that we don't support such feature? |
We could, but it'd just be going back to saying "we don't support representations that are folders". Which I suppose is a 'fine' but it does add complexity in other areas like Harmony and Silhoutte (and Mocha?) where we need to be dealing with zipped representations instead. And as soon as a 'workfile' becomes a ZIP that we potentially need to access from outside the applications, e.g. on Deadline render farm we get into tricky situations there. Because then suddenly Deadline render needs to know that the path it got passed is a Something that would be solved if the representations would be allowed to be folders too. Moving some of the complexity to core, but simplify the implementation across multiple host integrations and addons. It's mostly a decision to be made at this point: Do we allow a representation to be a folder?
|
We can allow representation to be a folder, but paths (which is metadata of representation) must contain paths to all files. How that paths thing affects what you did here? Are you accessing the paths directly in loader? Because if you've used My question regarding supporting was related to |
Again, as stated - with this PR this already works. It's functional. But I'd need to investigate how it'd differ if the integrator were to register the files themselves individually (whilst keeping the representation path itself to be the folder path). |
Ok, then change integrator to use all subfiles for representation paths, if representation source path is folder (should be change in |
@antirotor have you considered this use case for representation traits? |
Changelog Description
Support integrating a folder (directory) as files for a representation
Additional info
By allowing this, we can for example integrate BorisFX Silhouette workfiles (for which its project's are folders instead of files)
Fix ynput/ayon-silhouette#7
With these changes it is possible to integrate the workfile, register the representation, etc.
However, there may be follow up issues:
The publisher folder will have an extension
Note: In Silhouette we're 'rather lucky' because the workfile project folders happen to have an extension
.sfx
and hence the 'integrated' folder will also have the extension and it matches 🥳 - however, I'm quite sure that the logic here still requires the integrated folder to also get an extension (due to the templating system). Which is technically fine, but something to be aware of. You likely couldn't turn it into a published folder namedmyfile/
because it'll definitely try and put some extension to it (which is what the love of representations has been about all this time)Testing notes: