From 80b6ef981a5bc43bf2f2eea5ce06895057472a9a Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Tue, 2 Aug 2022 18:13:39 +0200 Subject: [PATCH 1/8] OP-3684 - fix for new publisher New publisher expects frames in file names in '.0000.' format, AE by default provides ('_0000.'). Locally rendered files need to be renamed to appropriate format. --- .../plugins/publish/extract_local_render.py | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py b/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py index 7323a0b125..67a89ba9df 100644 --- a/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py +++ b/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py @@ -1,7 +1,8 @@ import os import sys import six - +import re +import shutil import openpype.api from openpype.hosts.aftereffects.api import get_stub @@ -22,15 +23,26 @@ class ExtractLocalRender(openpype.api.Extractor): # pull file name from Render Queue Output module render_q = stub.get_render_info() stub.render(staging_dir) + render_q_file_name = render_q.file_name if not render_q: raise ValueError("No file extension set in Render Queue") - _, ext = os.path.splitext(os.path.basename(render_q.file_name)) + _, ext = os.path.splitext(os.path.basename(render_q_file_name)) ext = ext[1:] + replace_frames_format = self._get_replace_format(render_q_file_name) + first_file_path = None files = [] - self.log.info("files::{}".format(os.listdir(staging_dir))) for file_name in os.listdir(staging_dir): + _, found_ext = os.path.splitext(file_name) + if found_ext[1:] != ext: + continue + + if replace_frames_format: + file_name = self._translate_frames(file_name, + replace_frames_format, + staging_dir) + files.append(file_name) if first_file_path is None: first_file_path = os.path.join(staging_dir, @@ -78,3 +90,23 @@ class ExtractLocalRender(openpype.api.Extractor): "stagingDir": staging_dir, "tags": ["thumbnail"] }) + + def _translate_frames(self, file_name, replace_frames_format, staging_dir): + orig_file_name = file_name + + found_frames = re.search(replace_frames_format, file_name) + if found_frames: + new_frames = found_frames.group(0).replace('_', '.') + file_name = file_name.replace(found_frames.group(0), new_frames) + shutil.move(os.path.join(staging_dir, orig_file_name), + os.path.join(staging_dir, file_name)) + + return file_name + + def _get_replace_format(self, file_name): + # replace delimiter for frames to one integrate is expecting (.0000.) + # returns frame format to be replaced + hashes_found = re.search(r"(_%5B[#]*%5D.)", file_name) + if hashes_found: + hashes = re.sub("[^#]", '', hashes_found.group(0)) + return "_[0-9]{{{0}}}.".format(len(hashes)) From 0761ba4bc3b029cc5a130f3cde5dcedebefa0d7a Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 13:34:24 +0200 Subject: [PATCH 2/8] OP-3684 - fix output compare for automatic testing --- tests/lib/testing_classes.py | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/lib/testing_classes.py b/tests/lib/testing_classes.py index f991f02227..aa366cd005 100644 --- a/tests/lib/testing_classes.py +++ b/tests/lib/testing_classes.py @@ -314,30 +314,21 @@ class PublishTest(ModuleUnitTest): Compares only presence, not size nor content! """ - published_dir_base = download_test_data - published_dir = os.path.join(output_folder_url, - self.PROJECT, - self.ASSET, - self.TASK, - "**") - expected_dir_base = os.path.join(published_dir_base, + published_dir_base = output_folder_url + expected_dir_base = os.path.join(download_test_data, "expected") - expected_dir = os.path.join(expected_dir_base, - self.PROJECT, - self.ASSET, - self.TASK, - "**") - print("Comparing published:'{}' : expected:'{}'".format(published_dir, - expected_dir)) + + print("Comparing published:'{}' : expected:'{}'".format(published_dir_base, + expected_dir_base)) published = set(f.replace(published_dir_base, '') for f in - glob.glob(published_dir, recursive=True) if + glob.glob(published_dir_base + "\\**", recursive=True) if f != published_dir_base and os.path.exists(f)) expected = set(f.replace(expected_dir_base, '') for f in - glob.glob(expected_dir, recursive=True) if + glob.glob(expected_dir_base + "\\**", recursive=True) if f != expected_dir_base and os.path.exists(f)) - not_matched = expected.difference(published) - assert not not_matched, "Missing {} files".format(not_matched) + not_matched = expected.symmetric_difference(published) + assert not not_matched, "Missing {} files".format("\n".join(sorted(not_matched))) class HostFixtures(PublishTest): From 67b9946f2057fb44133edfd2fa70fddfe9ce2de3 Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 13:36:20 +0200 Subject: [PATCH 3/8] OP-3684 - added new testing class for multiframe AE publish Previous test published only single frame, didn't catch issue in new integrate. --- ...test_publish_in_aftereffects_multiframe.py | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/integration/hosts/aftereffects/test_publish_in_aftereffects_multiframe.py diff --git a/tests/integration/hosts/aftereffects/test_publish_in_aftereffects_multiframe.py b/tests/integration/hosts/aftereffects/test_publish_in_aftereffects_multiframe.py new file mode 100644 index 0000000000..c882e0f9b2 --- /dev/null +++ b/tests/integration/hosts/aftereffects/test_publish_in_aftereffects_multiframe.py @@ -0,0 +1,64 @@ +import logging + +from tests.lib.assert_classes import DBAssert +from tests.integration.hosts.aftereffects.lib import AfterEffectsTestClass + +log = logging.getLogger("test_publish_in_aftereffects") + + +class TestPublishInAfterEffects(AfterEffectsTestClass): + """Basic test case for publishing in AfterEffects + + Should publish 5 frames + """ + PERSIST = True + + TEST_FILES = [ + ("12aSDRjthn4X3yw83gz_0FZJcRRiVDEYT", + "test_aftereffects_publish_multiframe.zip", + "") + ] + + APP = "aftereffects" + APP_VARIANT = "" + + APP_NAME = "{}/{}".format(APP, APP_VARIANT) + + TIMEOUT = 120 # publish timeout + + def test_db_asserts(self, dbcon, publish_finished): + """Host and input data dependent expected results in DB.""" + print("test_db_asserts") + failures = [] + + failures.append(DBAssert.count_of_types(dbcon, "version", 2)) + + failures.append( + DBAssert.count_of_types(dbcon, "version", 0, name={"$ne": 1})) + + failures.append( + DBAssert.count_of_types(dbcon, "subset", 1, + name="imageMainBackgroundcopy")) + + failures.append( + DBAssert.count_of_types(dbcon, "subset", 1, + name="workfileTest_task")) + + failures.append( + DBAssert.count_of_types(dbcon, "subset", 1, + name="reviewTesttask")) + + failures.append( + DBAssert.count_of_types(dbcon, "representation", 4)) + + additional_args = {"context.subset": "renderTestTaskDefault", + "context.ext": "png"} + failures.append( + DBAssert.count_of_types(dbcon, "representation", 1, + additional_args=additional_args)) + + assert not any(failures) + + +if __name__ == "__main__": + test_case = TestPublishInAfterEffects() From ef60744d9c1aedea3f442792733d844ed0d845b7 Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 14:58:36 +0200 Subject: [PATCH 4/8] OP-3684 - added default to Integrate Setting to skip render.farm New publisher requires main family as 'render', so there will be need to skip 'render.farm' which should not be integrated during initial publish. (Currently only affecting AE.) --- openpype/plugins/publish/integrate.py | 11 ++++++----- .../settings/defaults/project_settings/global.json | 11 ++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index d817595888..70ab9f611e 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -167,7 +167,7 @@ class IntegrateAsset(pyblish.api.InstancePlugin): skip_host_families = [] def process(self, instance): - if self._temp_skip_instance_by_settings(instance): + if self.skip_instance_by_settings(instance): return # Mark instance as processed for legacy integrator @@ -203,11 +203,12 @@ class IntegrateAsset(pyblish.api.InstancePlugin): # the try, except. file_transactions.finalize() - def _temp_skip_instance_by_settings(self, instance): - """Decide if instance will be processed with new or legacy integrator. + def skip_instance_by_settings(self, instance): + """Decide if instance will be processed with new integrator. - This is temporary solution until we test all usecases with new (this) - integrator plugin. + This might be temporary solution for broken publishing for any families + (therefore it should fallback into legacy publish plugin) OR this + could replace 'exclude_families' in legacy plugin (host is required). """ host_name = instance.context.data["hostName"] diff --git a/openpype/settings/defaults/project_settings/global.json b/openpype/settings/defaults/project_settings/global.json index e509db2791..d349066924 100644 --- a/openpype/settings/defaults/project_settings/global.json +++ b/openpype/settings/defaults/project_settings/global.json @@ -225,7 +225,16 @@ ] }, "IntegrateAsset": { - "skip_host_families": [] + "skip_host_families": [ + { + "host": [ + "aftereffects" + ], + "families": [ + "render.farm" + ] + } + ] }, "IntegrateHeroVersion": { "enabled": true, From d0ac6bc9b0b55cbe4897d9ba129412316202d6eb Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 15:08:13 +0200 Subject: [PATCH 5/8] OP-3684 - Hound --- tests/lib/testing_classes.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/lib/testing_classes.py b/tests/lib/testing_classes.py index aa366cd005..2b4d7deb48 100644 --- a/tests/lib/testing_classes.py +++ b/tests/lib/testing_classes.py @@ -318,17 +318,18 @@ class PublishTest(ModuleUnitTest): expected_dir_base = os.path.join(download_test_data, "expected") - print("Comparing published:'{}' : expected:'{}'".format(published_dir_base, - expected_dir_base)) + print("Comparing published:'{}' : expected:'{}'".format( + published_dir_base, expected_dir_base)) published = set(f.replace(published_dir_base, '') for f in - glob.glob(published_dir_base + "\\**", recursive=True) if - f != published_dir_base and os.path.exists(f)) + glob.glob(published_dir_base + "\\**", recursive=True) + if f != published_dir_base and os.path.exists(f)) expected = set(f.replace(expected_dir_base, '') for f in - glob.glob(expected_dir_base + "\\**", recursive=True) if - f != expected_dir_base and os.path.exists(f)) + glob.glob(expected_dir_base + "\\**", recursive=True) + if f != expected_dir_base and os.path.exists(f)) not_matched = expected.symmetric_difference(published) - assert not not_matched, "Missing {} files".format("\n".join(sorted(not_matched))) + assert not not_matched, "Missing {} files".format( + "\n".join(sorted(not_matched))) class HostFixtures(PublishTest): From 99469a14438665595fafc21bdc517c083d76bd2c Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 16:24:50 +0200 Subject: [PATCH 6/8] OP-3684 - revert - added default to Integrate Setting to skip render.farm" This reverts commit ef60744d Not necessary, better to use `instance.data["farm"]` --- openpype/plugins/publish/integrate.py | 11 +++++------ .../settings/defaults/project_settings/global.json | 11 +---------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 70ab9f611e..d817595888 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -167,7 +167,7 @@ class IntegrateAsset(pyblish.api.InstancePlugin): skip_host_families = [] def process(self, instance): - if self.skip_instance_by_settings(instance): + if self._temp_skip_instance_by_settings(instance): return # Mark instance as processed for legacy integrator @@ -203,12 +203,11 @@ class IntegrateAsset(pyblish.api.InstancePlugin): # the try, except. file_transactions.finalize() - def skip_instance_by_settings(self, instance): - """Decide if instance will be processed with new integrator. + def _temp_skip_instance_by_settings(self, instance): + """Decide if instance will be processed with new or legacy integrator. - This might be temporary solution for broken publishing for any families - (therefore it should fallback into legacy publish plugin) OR this - could replace 'exclude_families' in legacy plugin (host is required). + This is temporary solution until we test all usecases with new (this) + integrator plugin. """ host_name = instance.context.data["hostName"] diff --git a/openpype/settings/defaults/project_settings/global.json b/openpype/settings/defaults/project_settings/global.json index d349066924..e509db2791 100644 --- a/openpype/settings/defaults/project_settings/global.json +++ b/openpype/settings/defaults/project_settings/global.json @@ -225,16 +225,7 @@ ] }, "IntegrateAsset": { - "skip_host_families": [ - { - "host": [ - "aftereffects" - ], - "families": [ - "render.farm" - ] - } - ] + "skip_host_families": [] }, "IntegrateHeroVersion": { "enabled": true, From bab5629e35736d94c01c012b0e5aee79fe95ba71 Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 16:26:46 +0200 Subject: [PATCH 7/8] OP-3684 - use instance.data["farm"] to skip local integrate No Settings necessary, instance itself should hold if it is targetted for farm (eg. not locally integrated.) --- .../hosts/aftereffects/plugins/publish/collect_render.py | 5 +++-- openpype/pipeline/publish/abstract_collect_render.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/openpype/hosts/aftereffects/plugins/publish/collect_render.py b/openpype/hosts/aftereffects/plugins/publish/collect_render.py index bb199a61f7..d444ead6dc 100644 --- a/openpype/hosts/aftereffects/plugins/publish/collect_render.py +++ b/openpype/hosts/aftereffects/plugins/publish/collect_render.py @@ -102,7 +102,6 @@ class CollectAERender(publish.AbstractCollectRender): attachTo=False, setMembers='', publish=True, - renderer='aerender', name=subset_name, resolutionWidth=render_q.width, resolutionHeight=render_q.height, @@ -113,7 +112,6 @@ class CollectAERender(publish.AbstractCollectRender): frameStart=frame_start, frameEnd=frame_end, frameStep=1, - toBeRenderedOn='deadline', fps=fps, app_version=app_version, publish_attributes=inst.data.get("publish_attributes", {}), @@ -138,6 +136,9 @@ class CollectAERender(publish.AbstractCollectRender): fam = "render.farm" if fam not in instance.families: instance.families.append(fam) + instance.toBeRenderedOn = "deadline" + instance.renderer = "aerender" + instance.farm = True # to skip integrate instances.append(instance) instances_to_remove.append(inst) diff --git a/openpype/pipeline/publish/abstract_collect_render.py b/openpype/pipeline/publish/abstract_collect_render.py index 2e537227c3..ccb2415346 100644 --- a/openpype/pipeline/publish/abstract_collect_render.py +++ b/openpype/pipeline/publish/abstract_collect_render.py @@ -63,6 +63,8 @@ class RenderInstance(object): family = attr.ib(default="renderlayer") families = attr.ib(default=["renderlayer"]) # list of families + # True if should be rendered on farm, eg not integrate + farm = attr.ib(default=False) # format settings multipartExr = attr.ib(default=False) # flag for multipart exrs From 7cfd9624a31b16f8bfff52684bacc7b9366cb925 Mon Sep 17 00:00:00 2001 From: Petr Kalis Date: Wed, 3 Aug 2022 17:39:35 +0200 Subject: [PATCH 8/8] "OP-3684 - revert - fix for new publisher" This reverts commit 80b6ef98 Made obsolete by https://github.com/pypeclub/OpenPype/pull/3611 --- .../plugins/publish/extract_local_render.py | 38 ++----------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py b/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py index 67a89ba9df..7323a0b125 100644 --- a/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py +++ b/openpype/hosts/aftereffects/plugins/publish/extract_local_render.py @@ -1,8 +1,7 @@ import os import sys import six -import re -import shutil + import openpype.api from openpype.hosts.aftereffects.api import get_stub @@ -23,26 +22,15 @@ class ExtractLocalRender(openpype.api.Extractor): # pull file name from Render Queue Output module render_q = stub.get_render_info() stub.render(staging_dir) - render_q_file_name = render_q.file_name if not render_q: raise ValueError("No file extension set in Render Queue") - _, ext = os.path.splitext(os.path.basename(render_q_file_name)) + _, ext = os.path.splitext(os.path.basename(render_q.file_name)) ext = ext[1:] - replace_frames_format = self._get_replace_format(render_q_file_name) - first_file_path = None files = [] + self.log.info("files::{}".format(os.listdir(staging_dir))) for file_name in os.listdir(staging_dir): - _, found_ext = os.path.splitext(file_name) - if found_ext[1:] != ext: - continue - - if replace_frames_format: - file_name = self._translate_frames(file_name, - replace_frames_format, - staging_dir) - files.append(file_name) if first_file_path is None: first_file_path = os.path.join(staging_dir, @@ -90,23 +78,3 @@ class ExtractLocalRender(openpype.api.Extractor): "stagingDir": staging_dir, "tags": ["thumbnail"] }) - - def _translate_frames(self, file_name, replace_frames_format, staging_dir): - orig_file_name = file_name - - found_frames = re.search(replace_frames_format, file_name) - if found_frames: - new_frames = found_frames.group(0).replace('_', '.') - file_name = file_name.replace(found_frames.group(0), new_frames) - shutil.move(os.path.join(staging_dir, orig_file_name), - os.path.join(staging_dir, file_name)) - - return file_name - - def _get_replace_format(self, file_name): - # replace delimiter for frames to one integrate is expecting (.0000.) - # returns frame format to be replaced - hashes_found = re.search(r"(_%5B[#]*%5D.)", file_name) - if hashes_found: - hashes = re.sub("[^#]", '', hashes_found.group(0)) - return "_[0-9]{{{0}}}.".format(len(hashes))