From fe58cb3c7cc4520b61c8b73c166f6694c5d29e91 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Mon, 3 Jul 2023 13:42:00 +0200 Subject: [PATCH] Enhancement: More descriptive error messages for Loaders (#5227) * More descriptive error messages for Loaders * Restructure error reporting --- openpype/pipeline/load/__init__.py | 4 ++ openpype/pipeline/load/utils.py | 37 +++++++++++++++---- .../tools/sceneinventory/switch_dialog.py | 24 +++++++++--- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/openpype/pipeline/load/__init__.py b/openpype/pipeline/load/__init__.py index e9ac0df924..7320a9f0e8 100644 --- a/openpype/pipeline/load/__init__.py +++ b/openpype/pipeline/load/__init__.py @@ -4,6 +4,8 @@ from .utils import ( LoadError, IncompatibleLoaderError, InvalidRepresentationContext, + LoaderSwitchNotImplementedError, + LoaderNotFoundError, get_repres_contexts, get_contexts_for_repre_docs, @@ -55,6 +57,8 @@ __all__ = ( "LoadError", "IncompatibleLoaderError", "InvalidRepresentationContext", + "LoaderSwitchNotImplementedError", + "LoaderNotFoundError", "get_repres_contexts", "get_contexts_for_repre_docs", diff --git a/openpype/pipeline/load/utils.py b/openpype/pipeline/load/utils.py index fefdb8537b..2c40280ccd 100644 --- a/openpype/pipeline/load/utils.py +++ b/openpype/pipeline/load/utils.py @@ -79,6 +79,16 @@ class InvalidRepresentationContext(ValueError): pass +class LoaderSwitchNotImplementedError(NotImplementedError): + """Error when `switch` is used with Loader that has no implementation.""" + pass + + +class LoaderNotFoundError(RuntimeError): + """Error when Loader plugin for a loader name is not found.""" + pass + + def get_repres_contexts(representation_ids, dbcon=None): """Return parenthood context for representation. @@ -432,7 +442,10 @@ def remove_container(container): Loader = _get_container_loader(container) if not Loader: - raise RuntimeError("Can't remove container. See log for details.") + raise LoaderNotFoundError( + "Can't remove container because loader '{}' was not found." + .format(container.get("loader")) + ) loader = Loader(get_representation_context(container["representation"])) return loader.remove(container) @@ -480,7 +493,10 @@ def update_container(container, version=-1): # Run update on the Loader for this container Loader = _get_container_loader(container) if not Loader: - raise RuntimeError("Can't update container. See log for details.") + raise LoaderNotFoundError( + "Can't update container because loader '{}' was not found." + .format(container.get("loader")) + ) loader = Loader(get_representation_context(container["representation"])) return loader.update(container, new_representation) @@ -502,15 +518,18 @@ def switch_container(container, representation, loader_plugin=None): loader_plugin = _get_container_loader(container) if not loader_plugin: - raise RuntimeError("Can't switch container. See log for details.") + raise LoaderNotFoundError( + "Can't switch container because loader '{}' was not found." + .format(container.get("loader")) + ) if not hasattr(loader_plugin, "switch"): # Backwards compatibility (classes without switch support # might be better to just have "switch" raise NotImplementedError # on the base class of Loader\ - raise RuntimeError("Loader '{}' does not support 'switch'".format( - loader_plugin.label - )) + raise LoaderSwitchNotImplementedError( + "Loader {} does not support 'switch'".format(loader_plugin.label) + ) # Get the new representation to switch to project_name = legacy_io.active_project() @@ -520,7 +539,11 @@ def switch_container(container, representation, loader_plugin=None): new_context = get_representation_context(new_representation) if not is_compatible_loader(loader_plugin, new_context): - raise AssertionError("Must be compatible Loader") + raise IncompatibleLoaderError( + "Loader {} is incompatible with {}".format( + loader_plugin.__name__, new_context["subset"]["name"] + ) + ) loader = loader_plugin(new_context) diff --git a/openpype/tools/sceneinventory/switch_dialog.py b/openpype/tools/sceneinventory/switch_dialog.py index 4aaad38bbc..ce2272df57 100644 --- a/openpype/tools/sceneinventory/switch_dialog.py +++ b/openpype/tools/sceneinventory/switch_dialog.py @@ -19,6 +19,9 @@ from openpype.pipeline.load import ( switch_container, get_repres_contexts, loaders_from_repre_context, + LoaderSwitchNotImplementedError, + IncompatibleLoaderError, + LoaderNotFoundError ) from .widgets import ( @@ -1298,19 +1301,28 @@ class SwitchAssetDialog(QtWidgets.QDialog): else: repre_doc = repres_by_name[container_repre_name] + error = None try: switch_container(container, repre_doc, loader) + except ( + LoaderSwitchNotImplementedError, + IncompatibleLoaderError, + LoaderNotFoundError, + ) as exc: + error = str(exc) except Exception: - msg = ( + error = ( + "Switch asset failed. " + "Search console log for more details." + ) + if error is not None: + log.warning(( "Couldn't switch asset." "See traceback for more information." - ) - log.warning(msg, exc_info=True) + ), exc_info=True) dialog = QtWidgets.QMessageBox(self) dialog.setWindowTitle("Switch asset failed") - dialog.setText( - "Switch asset failed. Search console log for more details" - ) + dialog.setText(error) dialog.exec_() self.switched.emit()