From 2e0fe9335151c6b7cdc9d25011216ca3b2705f5d Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:16:46 +0200 Subject: [PATCH 01/13] use KnownPublishError instead of assertions --- openpype/plugins/publish/integrate.py | 42 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 8ab508adc9f..e87538a5a43 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -517,14 +517,16 @@ def prepare_representation(self, repre, # pre-flight validations if repre["ext"].startswith("."): - raise ValueError("Extension must not start with a dot '.': " - "{}".format(repre["ext"])) + raise KnownPublishError(( + "Extension must not start with a dot '.': {}" + ).format(repre["ext"])) if repre.get("transfers"): - raise ValueError("Representation is not allowed to have transfers" - "data before integration. They are computed in " - "the integrator" - "Got: {}".format(repre["transfers"])) + raise KnownPublishError(( + "Representation is not allowed to have transfers" + "data before integration. They are computed in " + "the integrator. Got: {}" + ).format(repre["transfers"])) # create template data for Anatomy template_data = copy.deepcopy(instance.data["anatomyData"]) @@ -563,8 +565,9 @@ def prepare_representation(self, repre, "{}".format(instance_stagingdir)) stagingdir = instance_stagingdir if not stagingdir: - raise ValueError("No staging directory set for representation: " - "{}".format(repre)) + raise KnownPublishError( + "No staging directory set for representation: {}".format(repre) + ) self.log.debug("Anatomy template name: {}".format(template_name)) anatomy = instance.context.data['anatomy'] @@ -574,9 +577,8 @@ def prepare_representation(self, repre, is_sequence_representation = isinstance(files, (list, tuple)) if is_sequence_representation: # Collection of files (sequence) - assert not any(os.path.isabs(fname) for fname in files), ( - "Given file names contain full paths" - ) + if any(os.path.isabs(fname) for fname in files): + raise KnownPublishError("Given file names contain full paths") src_collection = assemble(files) @@ -632,9 +634,11 @@ def prepare_representation(self, repre, dst_collection.indexes.clear() dst_collection.indexes.update(set(destination_indexes)) dst_collection.padding = destination_padding - assert ( - len(src_collection.indexes) == len(dst_collection.indexes) - ), "This is a bug" + if len(src_collection.indexes) != len(dst_collection.indexes): + raise KnownPublishError(( + "This is a bug. Source sequence frames length" + " does not match integration frames length" + )) # Multiple file transfers transfers = [] @@ -645,9 +649,13 @@ def prepare_representation(self, repre, else: # Single file fname = files - assert not os.path.isabs(fname), ( - "Given file name is a full path" - ) + if os.path.isabs(fname): + self.log.error( + "Filename in representation is filepath {}".format(fname) + ) + raise KnownPublishError( + "This is a bug. Representation file name is full path" + ) # Manage anatomy template data template_data.pop("frame", None) From 1bb9b27c7ff5a8c7d0a8fb4c1e631e5e6d33be1d Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:17:07 +0200 Subject: [PATCH 02/13] simplified staging dir resolving --- openpype/plugins/publish/integrate.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index e87538a5a43..fdf5b21a6b9 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -556,14 +556,15 @@ def prepare_representation(self, repre, continue template_data[anatomy_key] = value - if repre.get('stagingDir'): - stagingdir = repre['stagingDir'] - else: + stagingdir = repre.get("stagingDir") + if not stagingdir: # Fall back to instance staging dir if not explicitly # set for representation in the instance - self.log.debug("Representation uses instance staging dir: " - "{}".format(instance_stagingdir)) + self.log.debug(( + "Representation uses instance staging dir: {}" + ).format(instance_stagingdir)) stagingdir = instance_stagingdir + if not stagingdir: raise KnownPublishError( "No staging directory set for representation: {}".format(repre) From 89d49533e4f15b3e055be9d01250780abb1bc199 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:17:56 +0200 Subject: [PATCH 03/13] add the values only if they are not 'None' --- openpype/plugins/publish/integrate.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index fdf5b21a6b9..87058dd2dab 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -686,9 +686,8 @@ def prepare_representation(self, repre, # Also add these values to the context even if not used by the # destination template value = template_data.get(key) - if not value: - continue - repre_context[key] = template_data[key] + if value is not None: + repre_context[key] = value # Explicitly store the full list even though template data might # have a different value because it uses just a single udim tile From 5272907504aa4b6e825d715dd7b9c1714f6fb85b Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:18:34 +0200 Subject: [PATCH 04/13] import source_hash directly --- openpype/plugins/publish/integrate.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 87058dd2dab..a5f5a66091e 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -9,12 +9,12 @@ from pymongo import DeleteMany, ReplaceOne, InsertOne, UpdateOne import pyblish.api -import openpype.api from openpype.client import ( get_representations, get_subset_by_name, get_version_by_name, ) +from openype.lib import source_hash from openpype.lib.profiles_filtering import filter_profiles from openpype.lib.file_transaction import FileTransaction from openpype.pipeline import legacy_io @@ -834,6 +834,7 @@ def _get_template_name_profiles(self, instance): def get_profile_filter_criteria(self, instance): """Return filter criteria for `filter_profiles`""" + # Anatomy data is pre-filled by Collectors anatomy_data = instance.data["anatomyData"] @@ -864,6 +865,7 @@ def get_rootless_path(self, anatomy, path): path: modified path if possible, or unmodified path + warning logged """ + success, rootless_path = anatomy.find_root_template_from_path(path) if success: path = rootless_path @@ -885,6 +887,7 @@ def get_files_info(self, destinations, sites, anatomy): output_resources: array of dictionaries to be added to 'files' key in representation """ + file_infos = [] for file_path in destinations: file_info = self.prepare_file_info(file_path, anatomy, sites=sites) @@ -904,10 +907,11 @@ def prepare_file_info(self, path, anatomy, sites): Returns: dict: file info dictionary """ + return { "_id": ObjectId(), "path": self.get_rootless_path(anatomy, path), "size": os.path.getsize(path), - "hash": openpype.api.source_hash(path), + "hash": source_hash(path), "sites": sites } From 0c061c50276ac68ead8b7d3918b007e65ab543e8 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:26:38 +0200 Subject: [PATCH 05/13] added "output" to representation context keys to auto fill it to context --- openpype/plugins/publish/integrate.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index a5f5a66091e..52a5ea2bfcf 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -168,7 +168,7 @@ class IntegrateAsset(pyblish.api.InstancePlugin): # the database even if not used by the destination template db_representation_context_keys = [ "project", "asset", "task", "subset", "version", "representation", - "family", "hierarchy", "username" + "family", "hierarchy", "username", "output" ] skip_host_families = [] @@ -727,11 +727,6 @@ def prepare_representation(self, repre, "context": repre_context } - # todo: simplify/streamline which additional data makes its way into - # the representation context - if repre.get("outputName"): - representation["context"]["output"] = repre['outputName'] - if is_sequence_representation and repre.get("frameStart") is not None: representation['context']['frame'] = template_data["frame"] From 9875f68cf43fef06e4670c6a5c61f3b3d5c0dbb0 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:27:13 +0200 Subject: [PATCH 06/13] don't just check existence of key but also it's value when traversing repre and instance data --- openpype/plugins/publish/integrate.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 52a5ea2bfcf..f89e7b33ce7 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -548,13 +548,12 @@ def prepare_representation(self, repre, }.items(): # Allow to take value from representation # if not found also consider instance.data - if key in repre: - value = repre[key] - elif key in instance.data: - value = instance.data[key] - else: - continue - template_data[anatomy_key] = value + value = repre.get(key) + if value is None: + value = instance.data.get(key) + + if value is not None: + template_data[anatomy_key] = value stagingdir = repre.get("stagingDir") if not stagingdir: From 0be6d5b55c0266241d7960a9a33056762cf788c2 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:29:24 +0200 Subject: [PATCH 07/13] removed backwards compatibility comments which as it's not backwards compatibility --- openpype/plugins/publish/integrate.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index f89e7b33ce7..7dfd8e4cac6 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -700,14 +700,12 @@ def prepare_representation(self, repre, else: repre_id = ObjectId() - # Backwards compatibility: # Store first transferred destination as published path data - # todo: can we remove this? - # todo: We shouldn't change data that makes its way back into - # instance.data[] until we know the publish actually succeeded - # otherwise `published_path` might not actually be valid? + # - used primarily for reviews that are integrated to custom modules + # TODO we should probably store all integrated files + # related to the representation? published_path = transfers[0][1] - repre["published_path"] = published_path # Backwards compatibility + repre["published_path"] = published_path # todo: `repre` is not the actual `representation` entity # we should simplify/clarify difference between data above From 12af64dbc0ed7eb6b415d55bc472c81c917eff7b Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:30:34 +0200 Subject: [PATCH 08/13] use last frame instead of first frame for padding and don't look at source collection padding --- openpype/plugins/publish/integrate.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 7dfd8e4cac6..3a86f4b3738 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -78,12 +78,6 @@ def get_frame_padded(frame, padding): return "{frame:0{padding}d}".format(padding=padding, frame=frame) -def get_first_frame_padded(collection): - """Return first frame as padded number from `clique.Collection`""" - start_frame = next(iter(collection.indexes)) - return get_frame_padded(start_frame, padding=collection.padding) - - class IntegrateAsset(pyblish.api.InstancePlugin): """Register publish in the database and transfer files to destinations. @@ -588,7 +582,9 @@ def prepare_representation(self, repre, # differs from the collection we want to shift the destination # frame indices from the source collection. destination_indexes = list(src_collection.indexes) - destination_padding = len(get_first_frame_padded(src_collection)) + # Use last frame for minimum padding + # - that should cover both 'udim' and 'frame' minimum padding + destination_padding = len(str(destination_indexes[-1])) if repre.get("frameStart") is not None and not is_udim: index_frame_start = int(repre.get("frameStart")) From 6cab5917c4903df529429ad5e5bf209409426708 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:36:23 +0200 Subject: [PATCH 09/13] use template padding for frames if padding is bigger then minimum collection's padding --- openpype/plugins/publish/integrate.py | 41 +++++++++++++-------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 3a86f4b3738..7a9cee593b5 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -565,7 +565,8 @@ def prepare_representation(self, repre, self.log.debug("Anatomy template name: {}".format(template_name)) anatomy = instance.context.data['anatomy'] - template = os.path.normpath(anatomy.templates[template_name]["path"]) + publish_template_category = anatomy.templates[template_name] + template = os.path.normpath(publish_template_category["path"]) is_udim = bool(repre.get("udim")) is_sequence_representation = isinstance(files, (list, tuple)) @@ -585,27 +586,25 @@ def prepare_representation(self, repre, # Use last frame for minimum padding # - that should cover both 'udim' and 'frame' minimum padding destination_padding = len(str(destination_indexes[-1])) - if repre.get("frameStart") is not None and not is_udim: - index_frame_start = int(repre.get("frameStart")) - - render_template = anatomy.templates[template_name] - # todo: should we ALWAYS manage the frame padding even when not - # having `frameStart` set? - frame_start_padding = int( - render_template.get( - "frame_padding", - render_template.get("padding") - ) + if not is_udim: + # Change padding for frames if template has defined higher + # padding. + template_padding = int( + publish_template_category["frame_padding"] ) - - # Shift destination sequence to the start frame - src_start_frame = next(iter(src_collection.indexes)) - shift = index_frame_start - src_start_frame - if shift: - destination_indexes = [ - frame + shift for frame in destination_indexes - ] - destination_padding = frame_start_padding + if template_padding > destination_padding: + destination_padding = template_padding + + if repre.get("frameStart") is not None: + index_frame_start = int(repre.get("frameStart")) + + # Shift destination sequence to the start frame + src_start_frame = next(iter(src_collection.indexes)) + shift = index_frame_start - src_start_frame + if shift: + destination_indexes = [ + frame + shift for frame in destination_indexes + ] # To construct the destination template with anatomy we require # a Frame or UDIM tile set for the template data. We use the first From 3835695376ff87983124a9ac802b5ecffa5e0344 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:38:51 +0200 Subject: [PATCH 10/13] simplified recalculation of destination indexes --- openpype/plugins/publish/integrate.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 7a9cee593b5..0387196a8a9 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -577,11 +577,6 @@ def prepare_representation(self, repre, src_collection = assemble(files) - # If the representation has `frameStart` set it renumbers the - # frame indices of the published collection. It will start from - # that `frameStart` index instead. Thus if that frame start - # differs from the collection we want to shift the destination - # frame indices from the source collection. destination_indexes = list(src_collection.indexes) # Use last frame for minimum padding # - that should cover both 'udim' and 'frame' minimum padding @@ -595,16 +590,19 @@ def prepare_representation(self, repre, if template_padding > destination_padding: destination_padding = template_padding - if repre.get("frameStart") is not None: - index_frame_start = int(repre.get("frameStart")) - + # If the representation has `frameStart` set it renumbers the + # frame indices of the published collection. It will start from + # that `frameStart` index instead. Thus if that frame start + # differs from the collection we want to shift the destination + # frame indices from the source collection. + repre_frame_start = repre.get("frameStart") + if repre_frame_start is not None: + index_frame_start = int(repre["frameStart"]) # Shift destination sequence to the start frame - src_start_frame = next(iter(src_collection.indexes)) - shift = index_frame_start - src_start_frame - if shift: - destination_indexes = [ - frame + shift for frame in destination_indexes - ] + destination_indexes = [ + index_frame_start + idx + for idx in range(len(destination_indexes)) + ] # To construct the destination template with anatomy we require # a Frame or UDIM tile set for the template data. We use the first From 879df0a3a79121a2fe9472e89e99537fc24f2040 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:51:16 +0200 Subject: [PATCH 11/13] unify quotations --- openpype/plugins/publish/integrate.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 0387196a8a9..81a2190a213 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -526,7 +526,7 @@ def prepare_representation(self, repre, template_data = copy.deepcopy(instance.data["anatomyData"]) # required representation keys - files = repre['files'] + files = repre["files"] template_data["representation"] = repre["name"] template_data["ext"] = repre["ext"] @@ -564,11 +564,12 @@ def prepare_representation(self, repre, ) self.log.debug("Anatomy template name: {}".format(template_name)) - anatomy = instance.context.data['anatomy'] + anatomy = instance.context.data["anatomy"] publish_template_category = anatomy.templates[template_name] template = os.path.normpath(publish_template_category["path"]) is_udim = bool(repre.get("udim")) + is_sequence_representation = isinstance(files, (list, tuple)) if is_sequence_representation: # Collection of files (sequence) @@ -704,13 +705,13 @@ def prepare_representation(self, repre, # we should simplify/clarify difference between data above # and the actual representation entity for the database data = repre.get("data", {}) - data.update({'path': published_path, 'template': template}) + data.update({"path": published_path, "template": template}) representation = { "_id": repre_id, "schema": "openpype:representation-2.0", "type": "representation", "parent": version["_id"], - "name": repre['name'], + "name": repre["name"], "data": data, # Imprint shortcut to context for performance reasons. @@ -718,7 +719,7 @@ def prepare_representation(self, repre, } if is_sequence_representation and repre.get("frameStart") is not None: - representation['context']['frame'] = template_data["frame"] + representation["context"]["frame"] = template_data["frame"] return { "representation": representation, @@ -779,7 +780,7 @@ def create_version_data(self, instance): version_data[key] = instance.data[key] # Include instance.data[versionData] directly - version_data_instance = instance.data.get('versionData') + version_data_instance = instance.data.get("versionData") if version_data_instance: version_data.update(version_data_instance) From 74ad4a558d9574f85cfe852576b6fdc2d40641ad Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:51:24 +0200 Subject: [PATCH 12/13] fix typo in import --- openpype/plugins/publish/integrate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 81a2190a213..db55a17e59d 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -14,7 +14,7 @@ get_subset_by_name, get_version_by_name, ) -from openype.lib import source_hash +from openpype.lib import source_hash from openpype.lib.profiles_filtering import filter_profiles from openpype.lib.file_transaction import FileTransaction from openpype.pipeline import legacy_io From b5cdebe0707c9e4a9acccd16b6db92108ba8cca8 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Wed, 27 Jul 2022 13:56:39 +0200 Subject: [PATCH 13/13] make sure frame is filled durectly in sequence condition --- openpype/plugins/publish/integrate.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index db55a17e59d..c106649f2ab 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -621,6 +621,13 @@ def prepare_representation(self, repre, anatomy_filled = anatomy.format(template_data) template_filled = anatomy_filled[template_name]["path"] repre_context = template_filled.used_values + + # Make sure context contains frame + # NOTE: Frame would not be available only if template does not + # contain '{frame}' in template -> Do we want support it? + if not is_udim: + repre_context["frame"] = first_index_padded + self.log.debug("Template filled: {}".format(str(template_filled))) dst_collection = assemble([os.path.normpath(template_filled)]) @@ -718,9 +725,6 @@ def prepare_representation(self, repre, "context": repre_context } - if is_sequence_representation and repre.get("frameStart") is not None: - representation["context"]["frame"] = template_data["frame"] - return { "representation": representation, "anatomy_data": template_data,