-
Notifications
You must be signed in to change notification settings - Fork 34
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
Use product name templates for render products on farm publish #447
Changes from 6 commits
2d403b3
43f2fed
cd2d5f4
cfe8f08
8fa1317
bf46634
ad61ced
1ae71bb
6541dbe
f7aada3
b37b0ec
eb83310
61147d2
19c7c04
0bcf3d3
9012e52
cba100a
62870f2
871f6a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
get_current_project_name, | ||
get_representation_path, | ||
) | ||
from ayon_core.pipeline.create import get_product_name | ||
|
||
from ayon_core.lib import Logger | ||
from ayon_core.pipeline.publish import KnownPublishError | ||
from ayon_core.pipeline.farm.patterning import match_aov_pattern | ||
|
@@ -464,7 +466,9 @@ def create_instances_for_aov(instance, skeleton, aov_filter, | |
Args: | ||
instance (pyblish.api.Instance): Original instance. | ||
skeleton (dict): Skeleton instance data. | ||
aov_filter (dict): AOV filter. | ||
skip_integration_repre_list (list): skip | ||
do_not_add_review (bool): explicitly disable review | ||
|
||
Returns: | ||
list of pyblish.api.Instance: Instances created from | ||
|
@@ -526,10 +530,10 @@ def _create_instances_for_aov(instance, skeleton, aov_filter, additional_data, | |
instance (pyblish.api.Instance): Original instance. | ||
skeleton (dict): Skeleton data for instance (those needed) later | ||
by collector. | ||
additional_data (dict): .. | ||
additional_data (dict): ... | ||
skip_integration_repre_list (list): list of extensions that shouldn't | ||
be published | ||
do_not_addbe _review (bool): explicitly disable review | ||
do_not_add_review (bool): explicitly disable review | ||
|
||
|
||
Returns: | ||
|
@@ -539,68 +543,60 @@ def _create_instances_for_aov(instance, skeleton, aov_filter, additional_data, | |
ValueError: | ||
|
||
""" | ||
# TODO: this needs to be taking the task from context or instance | ||
task = os.environ["AYON_TASK_NAME"] | ||
task_name = instance.data['taskEntity']['name'] | ||
antirotor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
anatomy = instance.context.data["anatomy"] | ||
s_product_name = skeleton["productName"] | ||
src_product_name = skeleton["productName"] | ||
cameras = instance.data.get("cameras", []) | ||
exp_files = instance.data["expectedFiles"] | ||
expected_files = instance.data["expectedFiles"] | ||
log = Logger.get_logger("farm_publishing") | ||
|
||
instances = [] | ||
# go through AOVs in expected files | ||
for aov, files in exp_files[0].items(): | ||
cols, rem = clique.assemble(files) | ||
# we shouldn't have any reminders. And if we do, it should | ||
# be just one item for single frame renders. | ||
if not cols and rem: | ||
if len(rem) != 1: | ||
raise ValueError("Found multiple non related files " | ||
"to render, don't know what to do " | ||
"with them.") | ||
col = rem[0] | ||
ext = os.path.splitext(col)[1].lstrip(".") | ||
else: | ||
# but we really expect only one collection. | ||
# Nothing else make sense. | ||
if len(cols) != 1: | ||
raise ValueError("Only one image sequence type is expected.") # noqa: E501 | ||
ext = cols[0].tail.lstrip(".") | ||
col = list(cols[0]) | ||
for aov, files in expected_files[0].items(): | ||
collected_files = _collect_expected_files_for_aov(files) | ||
|
||
# get file path (use first from the list or single frame) | ||
expected_filepath = collected_files[0] if isinstance(collected_files, (list, tuple)) else collected_files | ||
|
||
dynamic_data = { | ||
"aov": aov, | ||
"renderlayer": instance.data.get("renderlayer"), | ||
} | ||
|
||
# find if camera is used in the file path | ||
camera = [cam for cam in cameras if cam in expected_filepath] | ||
|
||
# Is there just one camera matching? | ||
# TODO: this is not true, we can have multiple cameras in the scene | ||
# and we should be able to detect them all. Currently, we are | ||
# keeping the old behavior, taking the first one found. | ||
if camera: | ||
dynamic_data["camera"] = camera[0] | ||
|
||
product_name = get_product_name( | ||
project_name=instance.context.data["projectName"], | ||
task_name=task_name, | ||
task_type=instance.data['taskEntity']['taskType'], | ||
antirotor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
host_name=instance.context.data["hostName"], | ||
product_type=skeleton['productType'], | ||
antirotor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
dynamic_data=dynamic_data, | ||
variant=instance.data.get('variant', ''), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really want to fall back to an empty variant? |
||
project_settings=instance.context.data.get("project_settings"), | ||
) | ||
|
||
# create product name `<product type><Task><Product name>` | ||
# TODO refactor/remove me | ||
product_type = skeleton["productType"] | ||
if not s_product_name.startswith(product_type): | ||
# Group name isn't based on a product name template as it is | ||
# difficult to differentiate in the custom defined template | ||
# what part is the least common denominator. | ||
if not src_product_name.startswith(skeleton["productType"]): | ||
group_name = '{}{}{}{}{}'.format( | ||
product_type, | ||
task[0].upper(), task[1:], | ||
s_product_name[0].upper(), s_product_name[1:]) | ||
skeleton["productType"], | ||
task_name[0].upper(), task_name[1:], | ||
src_product_name[0].upper(), src_product_name[1:]) | ||
else: | ||
group_name = s_product_name | ||
|
||
# if there are multiple cameras, we need to add camera name | ||
expected_filepath = col[0] if isinstance(col, (list, tuple)) else col | ||
cams = [cam for cam in cameras if cam in expected_filepath] | ||
if cams: | ||
for cam in cams: | ||
if not aov: | ||
product_name = '{}_{}'.format(group_name, cam) | ||
elif not aov.startswith(cam): | ||
product_name = '{}_{}_{}'.format(group_name, cam, aov) | ||
else: | ||
product_name = "{}_{}".format(group_name, aov) | ||
else: | ||
if aov: | ||
product_name = '{}_{}'.format(group_name, aov) | ||
else: | ||
product_name = '{}'.format(group_name) | ||
group_name = src_product_name | ||
|
||
if isinstance(col, (list, tuple)): | ||
staging = os.path.dirname(col[0]) | ||
else: | ||
staging = os.path.dirname(col) | ||
staging = os.path.dirname(expected_filepath) | ||
|
||
try: | ||
staging = remap_source(staging, anatomy) | ||
|
@@ -611,20 +607,18 @@ def _create_instances_for_aov(instance, skeleton, aov_filter, additional_data, | |
|
||
app = os.environ.get("AYON_HOST_NAME", "") | ||
|
||
if isinstance(col, list): | ||
render_file_name = os.path.basename(col[0]) | ||
else: | ||
render_file_name = os.path.basename(col) | ||
aov_patterns = aov_filter | ||
render_file_name = os.path.basename(expected_filepath) | ||
|
||
aov_patterns = aov_filter | ||
preview = match_aov_pattern(app, aov_patterns, render_file_name) | ||
|
||
new_instance = deepcopy(skeleton) | ||
new_instance["productName"] = product_name | ||
new_instance["productGroup"] = group_name | ||
new_instance["aov"] = aov | ||
|
||
# toggle preview on if multipart is on | ||
# Because we cant query the multipartExr data member of each AOV we'll | ||
# Because we can't query the multipartExr data member of each AOV we'll | ||
# need to have hardcoded rule of excluding any renders with | ||
# "cryptomatte" in the file name from being a multipart EXR. This issue | ||
# happens with Redshift that forces Cryptomatte renders to be separate | ||
|
@@ -650,10 +644,9 @@ def _create_instances_for_aov(instance, skeleton, aov_filter, additional_data, | |
new_instance["review"] = True | ||
|
||
# create representation | ||
if isinstance(col, (list, tuple)): | ||
files = [os.path.basename(f) for f in col] | ||
else: | ||
files = os.path.basename(col) | ||
ext = os.path.splitext( | ||
os.path.basename(expected_filepath) | ||
)[1].lstrip(".") | ||
|
||
# Copy render product "colorspace" data to representation. | ||
colorspace = "" | ||
|
@@ -708,6 +701,36 @@ def _create_instances_for_aov(instance, skeleton, aov_filter, additional_data, | |
return instances | ||
|
||
|
||
def _collect_expected_files_for_aov(files): | ||
"""Collect expected files. | ||
|
||
Args: | ||
files (list): List of files. | ||
|
||
Returns: | ||
list or str: Collection of files or single file. | ||
|
||
Raises: | ||
ValueError: If there are multiple collections. | ||
|
||
""" | ||
cols, rem = clique.assemble(files) | ||
# we shouldn't have any reminders. And if we do, it should | ||
# be just one item for single frame renders. | ||
if not cols and rem: | ||
if len(rem) != 1: | ||
raise ValueError("Found multiple non related files " | ||
"to render, don't know what to do " | ||
"with them.") | ||
return rem[0] | ||
else: | ||
# but we really expect only one collection. | ||
# Nothing else make sense. | ||
if len(cols) != 1: | ||
raise ValueError("Only one image sequence type is expected.") # noqa: E501 | ||
return list(cols[0]) | ||
|
||
|
||
def get_resources(project_name, version_entity, extension=None): | ||
"""Get the files from the specific version. | ||
|
||
|
@@ -837,6 +860,8 @@ def create_skeleton_instance_cache(instance): | |
# map inputVersions `ObjectId` -> `str` so json supports it | ||
"inputVersions": list(map(str, data.get("inputVersions", []))), | ||
} | ||
if instance.data.get("renderlayer"): | ||
instance_skeleton_data["renderlayer"] = instance.data["renderlayer"] | ||
|
||
# skip locking version if we are creating v01 | ||
instance_version = data.get("version") # take this if exists | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -137,7 +137,7 @@ def fill_missing_task_entities(self, context, project_name): | |||||||||
folder_path_by_id = {} | ||||||||||
for instance in context: | ||||||||||
folder_entity = instance.data.get("folderEntity") | ||||||||||
# Skip if instnace does not have filled folder entity | ||||||||||
# Skip if instance does not have filled folder entity | ||||||||||
if not folder_entity: | ||||||||||
continue | ||||||||||
folder_id = folder_entity["id"] | ||||||||||
|
@@ -352,6 +352,17 @@ def fill_anatomy_data(self, context): | |||||||||
if resolution_height: | ||||||||||
anatomy_data["resolution_height"] = resolution_height | ||||||||||
|
||||||||||
# make render layer name (or ROP name) available in the | ||||||||||
# templates | ||||||||||
render_layer = instance.data.get("renderlayer") | ||||||||||
if render_layer: | ||||||||||
anatomy_data["renderlayer"] = render_layer | ||||||||||
|
||||||||||
# make AOV name if present available for templates | ||||||||||
aov = instance.data.get("aov") | ||||||||||
if aov: | ||||||||||
anatomy_data["aov"] = aov | ||||||||||
Comment on lines
+356
to
+365
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's "renderlayer" if not part of the product name already? This seems like an odd decision to allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you might want to have render layer as a part of the path (not the whole product name). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When doing so - what would you anticipate the product name be instead? Coming back to this from your PR description:
In practice a user should usually never see this. They see the product name or representation in the loader or the manager. Which raises the question - how come its this crucial in the published filepath which usually isn't something that is displayed to the artists anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is just an option to categorize things when you need to have access to them outside loaders/pipeline. We get those requests from time to time when you have part that is not integrated and you need to access the files directly. Then dropping the whole render layer folder to DCC with all of it's AOV sequences can speed things up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally agree that's valuable - but doesn't that mean we should just expose these as data on the representations that can be picked up by the Delivery action instead? Because I believe this currently requires the template to actually use If not and this is solely for "delivery" templates I think it could be nice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean to "force" it to context on representation even without it being used in the template so it can be picked up later on by delivery action? Like we are storing ayon-core/client/ayon_core/plugins/publish/integrate.py Lines 856 to 859 in 4d84ed8
frame somewhere)
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, similar to that. Not sure if that's "good design" - but that was indeed what I meant. I'd expect this to way more valuable for delivery templates than production templates since in production templates it may be likely you will have duplicity anyway since usually the AOV is part of the product name as well. |
||||||||||
|
||||||||||
pixel_aspect = instance.data.get("pixelAspect") | ||||||||||
if pixel_aspect: | ||||||||||
anatomy_data["pixel_aspect"] = float( | ||||||||||
|
@@ -378,7 +389,7 @@ def fill_anatomy_data(self, context): | |||||||||
)) | ||||||||||
|
||||||||||
def _fill_folder_data(self, instance, project_entity, anatomy_data): | ||||||||||
# QUESTION should we make sure that all folder data are poped if | ||||||||||
# QUESTION should we make sure that all folder data are popped if | ||||||||||
# folder data cannot be found? | ||||||||||
# - 'folder', 'hierarchy', 'parent', 'folder' | ||||||||||
folder_entity = instance.data.get("folderEntity") | ||||||||||
|
@@ -422,7 +433,7 @@ def _fill_folder_data(self, instance, project_entity, anatomy_data): | |||||||||
}) | ||||||||||
|
||||||||||
def _fill_task_data(self, instance, task_types_by_name, anatomy_data): | ||||||||||
# QUESTION should we make sure that all task data are poped if task | ||||||||||
# QUESTION should we make sure that all task data are popped if task | ||||||||||
# data cannot be resolved? | ||||||||||
# - 'task' | ||||||||||
|
||||||||||
|
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.
Sorry, what? Why is the ROP node's name used as the
renderlayer
? That seems odd?Isn't it much easier if a studio wants this for them to us
$OS
as part of theproductName
orsubset
attributes on the node itself so that it's editable by the studio, requires no additional changes?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.
Yes, but then you can't use it in anatomy data. One thing is product name, the other is to use it somewhere else in path. I agree that render layer is somewhat arbitrary name in Houdini context, but most of the DCCs are using render layer is one way or the other and I wouldn't spam the anatomy data with every DCC specific term.