From fe62979c978a3f97f1eeea29438ac417a29398fd Mon Sep 17 00:00:00 2001 From: Toke Jepsen Date: Wed, 31 Jul 2019 10:34:25 +0100 Subject: [PATCH] Use lib subprocess. - Unifies all subprocess calls. - Validates ffmpeg process. - Feedback the errored subprocess output. --- pype/api.py | 6 +++ pype/lib.py | 19 +++++++ pype/plugins/global/publish/extract_burnin.py | 53 +++++++++++-------- pype/plugins/global/publish/extract_jpeg.py | 10 ++-- pype/plugins/global/publish/extract_review.py | 21 ++------ 5 files changed, 67 insertions(+), 42 deletions(-) diff --git a/pype/api.py b/pype/api.py index 1ecbe3b36a..f20a0508e1 100644 --- a/pype/api.py +++ b/pype/api.py @@ -43,6 +43,9 @@ from .lib import ( get_avalon_project_template ) +# Special naming case for subprocess since its a built-in method. +from .lib import _subprocess as subprocess + __all__ = [ # plugin classes "Extractor", @@ -54,12 +57,14 @@ __all__ = [ # action "get_errored_instances_from_context", "RepairAction", + "RepairContextAction", "Logger", "ValidationException", # get contextual data + "version_up", "get_handle_irregular", "get_project_data", "get_asset_data", @@ -77,5 +82,6 @@ __all__ = [ "set_project_code", "get_data_hierarchical_attr", "get_avalon_project_template", + "subprocess" ] diff --git a/pype/lib.py b/pype/lib.py index e163cc14fc..15f248b554 100644 --- a/pype/lib.py +++ b/pype/lib.py @@ -4,6 +4,7 @@ import logging import importlib import itertools import contextlib +import subprocess from .vendor import pather from .vendor.pather.error import ParseError @@ -15,6 +16,24 @@ import avalon log = logging.getLogger(__name__) +# Special naming case for subprocess since its a built-in method. +def _subprocess(args): + """Convenience method for getting output errors for subprocess.""" + + proc = subprocess.Popen( + args, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + stdin=subprocess.PIPE, + env=os.environ + ) + + output = proc.communicate()[0] + + if proc.returncode != 0: + raise ValueError("\"{}\" was not successful: {}".format(args, output)) + + def get_handle_irregular(asset): data = asset["data"] handle_start = data.get("handle_start", 0) diff --git a/pype/plugins/global/publish/extract_burnin.py b/pype/plugins/global/publish/extract_burnin.py index 5f16cc91f2..0a60a987dc 100644 --- a/pype/plugins/global/publish/extract_burnin.py +++ b/pype/plugins/global/publish/extract_burnin.py @@ -1,7 +1,7 @@ import os -import subprocess -import pype.api import json + +import pype.api import pyblish @@ -67,25 +67,36 @@ class ExtractBurnin(pype.api.Extractor): "otio_burnin.py")) self.log.debug("__ scriptpath: {}".format(scriptpath)) - self.log.debug("__ EXE: {}".format(os.getenv("PYPE_PYTHON_EXE"))) - try: - p = subprocess.Popen( - [os.getenv("PYPE_PYTHON_EXE"), scriptpath, json_data] - ) - p.wait() - if not os.path.isfile(full_burnin_path): - raise RuntimeError("File not existing: {}".format(full_burnin_path)) - except Exception as e: - raise RuntimeError("Burnin script didn't work: `{}`".format(e)) + # Get executable. + executable = os.getenv("PYPE_PYTHON_EXE") - if os.path.exists(full_burnin_path): - repre_update = { - "files": movieFileBurnin, - "name": repre["name"] - } - instance.data["representations"][i].update(repre_update) + # There can be multiple paths in PYPE_PYTHON_EXE, in which case + # we just take first one. + if os.pathsep in executable: + executable = executable.split(os.pathsep)[0] - # removing the source mov file - os.remove(full_movie_path) - self.log.debug("Removed: `{}`".format(full_movie_path)) + self.log.debug("__ EXE: {}".format(executable)) + + args = [executable, scriptpath, json_data] + self.log.debug("Executing: {}".format(args)) + pype.api.subprocess(args) + + repre_update = { + "files": movieFileBurnin, + "name": repre["name"], + "tags": [x for x in repre["tags"] if x != "delete"] + } + instance.data["representations"][i].update(repre_update) + + # removing the source mov file + os.remove(full_movie_path) + self.log.debug("Removed: `{}`".format(full_movie_path)) + + # Remove any representations tagged for deletion. + for repre in instance.data["representations"]: + if "delete" in repre.get("tags", []): + self.log.debug("Removing representation: {}".format(repre)) + instance.data["representations"].remove(repre) + + self.log.debug(instance.data["representations"]) diff --git a/pype/plugins/global/publish/extract_jpeg.py b/pype/plugins/global/publish/extract_jpeg.py index 3c4c2e6775..8ea645798e 100644 --- a/pype/plugins/global/publish/extract_jpeg.py +++ b/pype/plugins/global/publish/extract_jpeg.py @@ -1,7 +1,8 @@ import os + import pyblish.api -import subprocess from pype.vendor import clique +import pype.api class ExtractJpegEXR(pyblish.api.InstancePlugin): @@ -20,7 +21,6 @@ class ExtractJpegEXR(pyblish.api.InstancePlugin): order = pyblish.api.ExtractorOrder families = ["imagesequence", "render", "write", "source"] - def process(self, instance): start = instance.data.get("startFrame") stagingdir = os.path.normpath(instance.data.get("stagingDir")) @@ -59,8 +59,10 @@ class ExtractJpegEXR(pyblish.api.InstancePlugin): jpeg_items.append(full_output_path) subprocess_jpeg = " ".join(jpeg_items) - sub_proc = subprocess.Popen(subprocess_jpeg) - sub_proc.wait() + + # run subprocess + self.log.debug("{}".format(subprocess_jpeg)) + pype.api.subprocess(subprocess_jpeg) if "representations" not in instance.data: instance.data["representations"] = [] diff --git a/pype/plugins/global/publish/extract_review.py b/pype/plugins/global/publish/extract_review.py index 1b66b4e9d2..82ea9d85dd 100644 --- a/pype/plugins/global/publish/extract_review.py +++ b/pype/plugins/global/publish/extract_review.py @@ -1,7 +1,8 @@ import os + import pyblish.api -import subprocess from pype.vendor import clique +import pype.api from pypeapp import config @@ -53,7 +54,7 @@ class ExtractReview(pyblish.api.InstancePlugin): ext = "mov" self.log.warning( "`ext` attribute not in output profile. Setting to default ext: `mov`") - + self.log.debug("instance.families: {}".format(instance.data['families'])) self.log.debug("profile.families: {}".format(profile['families'])) @@ -136,21 +137,7 @@ class ExtractReview(pyblish.api.InstancePlugin): # run subprocess self.log.debug("{}".format(subprcs_cmd)) - sub_proc = subprocess.Popen( - subprcs_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - stdin=subprocess.PIPE, - cwd=os.path.dirname(output_args[-1]) - ) - - output = sub_proc.communicate()[0] - - if not os.path.isfile(full_output_path): - raise ValueError( - "Quicktime wasn't created succesfully: " - "{}".format(output) - ) + pype.api.subprocess(subprcs_cmd) # create representation data repre_new.update({