From 05c87821941bf2dd51f50453fb5d2864a7419092 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 14:32:43 +0200 Subject: [PATCH 01/23] raise exception on collect/save/remove operations of creator --- openpype/pipeline/create/context.py | 85 +++++++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/openpype/pipeline/create/context.py b/openpype/pipeline/create/context.py index 4ec6d7bdad..6bc70f33ea 100644 --- a/openpype/pipeline/create/context.py +++ b/openpype/pipeline/create/context.py @@ -62,6 +62,22 @@ class HostMissRequiredMethod(Exception): super(HostMissRequiredMethod, self).__init__(msg) +class CreatorOperationFailed(Exception): + pass + + +class CreatorsCollectionFailed(CreatorOperationFailed): + pass + + +class CreatorsSaveFailed(CreatorOperationFailed): + pass + + +class CreatorsRemoveFailed(CreatorOperationFailed): + pass + + class InstanceMember: """Representation of instance member. @@ -1221,8 +1237,26 @@ class CreateContext: self._instances_by_id = {} # Collect instances + failed_creators = [] for creator in self.creators.values(): - creator.collect_instances() + try: + creator.collect_instances() + except: + failed_creators.append(creator) + self.log.warning( + "Collection of instances for creator {} ({}) failed".format( + creator.label, creator.identifier), + exc_info=True + ) + + if failed_creators: + joined_creators = ", ".join( + [creator.label for creator in failed_creators] + ) + + raise CreatorsCollectionFailed( + "Failed to collect instances of creators {}".format(joined_creators) + ) def execute_autocreators(self): """Execute discovered AutoCreator plugins. @@ -1315,16 +1349,35 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) - for identifier, cretor_instances in instances_by_identifier.items(): + failed_creators = [] + for identifier, creator_instances in instances_by_identifier.items(): update_list = [] - for instance in cretor_instances: + for instance in creator_instances: instance_changes = instance.changes() if instance_changes: update_list.append(UpdateData(instance, instance_changes)) creator = self.creators[identifier] if update_list: - creator.update_instances(update_list) + try: + creator.update_instances(update_list) + + except: + failed_creators.append(creator) + self.log.warning( + "Instances update of creator {} ({}) failed".format( + creator.label, creator.identifier), + exc_info=True + ) + + if failed_creators: + joined_creators = ", ".join( + [creator.label for creator in failed_creators] + ) + + raise CreatorsSaveFailed( + "Failed save changes of creators {}".format(joined_creators) + ) def remove_instances(self, instances): """Remove instances from context. @@ -1333,14 +1386,36 @@ class CreateContext: instances(list): Instances that should be removed from context. """ + instances_by_identifier = collections.defaultdict(list) for instance in instances: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) + failed_creators = [] for identifier, creator_instances in instances_by_identifier.items(): creator = self.creators.get(identifier) - creator.remove_instances(creator_instances) + try: + creator.remove_instances(creator_instances) + except: + failed_creators.append(creator) + self.log.warning( + "Instances removement of creator {} ({}) failed".format( + creator.label, creator.identifier), + exc_info=True + ) + + if failed_creators: + joined_creators = ", ".join( + [creator.label for creator in failed_creators] + ) + + raise CreatorsRemoveFailed( + "Failed to remove instances of creators {}".format( + joined_creators + ) + ) + def _get_publish_plugins_with_attr_for_family(self, family): """Publish plugin attributes for passed family. From 3f54214a1113bac1ab5b53e8a0f17078eba1ce6a Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 14:43:32 +0200 Subject: [PATCH 02/23] don't autotrigger save on controller reset --- openpype/tools/publisher/control.py | 3 --- openpype/tools/publisher/window.py | 41 ++++++++++++++++++----------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/openpype/tools/publisher/control.py b/openpype/tools/publisher/control.py index a340f8c1d2..1a15a71040 100644 --- a/openpype/tools/publisher/control.py +++ b/openpype/tools/publisher/control.py @@ -1662,11 +1662,8 @@ class PublisherController(BasePublisherController): def reset(self): """Reset everything related to creation and publishing.""" - # Stop publishing self.stop_publish() - self.save_changes() - self.host_is_valid = self._create_context.host_is_valid # Reset avalon context diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 39075d2489..7559b4a641 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -36,7 +36,7 @@ class PublisherWindow(QtWidgets.QDialog): footer_border = 8 publish_footer_spacer = 2 - def __init__(self, parent=None, controller=None, reset_on_show=None): + def __init__(self, parent=None, controller=None, reset_on_first_show=None): super(PublisherWindow, self).__init__(parent) self.setWindowTitle("OpenPype publisher") @@ -44,8 +44,8 @@ class PublisherWindow(QtWidgets.QDialog): icon = QtGui.QIcon(resources.get_openpype_icon_filepath()) self.setWindowIcon(icon) - if reset_on_show is None: - reset_on_show = True + if reset_on_first_show is None: + reset_on_first_show = True if parent is None: on_top_flag = QtCore.Qt.WindowStaysOnTopHint @@ -298,7 +298,8 @@ class PublisherWindow(QtWidgets.QDialog): self._controller = controller self._first_show = True - self._reset_on_show = reset_on_show + self._reset_on_first_show = reset_on_first_show + self._reset_on_show = True self._restart_timer = None self._publish_frame_visible = None @@ -314,6 +315,18 @@ class PublisherWindow(QtWidgets.QDialog): self._first_show = False self._on_first_show() + if not self._reset_on_show: + return + + self._reset_on_show = False + # Detach showing - give OS chance to draw the window + timer = QtCore.QTimer() + timer.setSingleShot(True) + timer.setInterval(1) + timer.timeout.connect(self._on_show_restart_timer) + self._restart_timer = timer + timer.start() + def resizeEvent(self, event): super(PublisherWindow, self).resizeEvent(event) self._update_publish_frame_rect() @@ -324,16 +337,7 @@ class PublisherWindow(QtWidgets.QDialog): def _on_first_show(self): self.resize(self.default_width, self.default_height) self.setStyleSheet(style.load_stylesheet()) - if not self._reset_on_show: - return - - # Detach showing - give OS chance to draw the window - timer = QtCore.QTimer() - timer.setSingleShot(True) - timer.setInterval(1) - timer.timeout.connect(self._on_show_restart_timer) - self._restart_timer = timer - timer.start() + self._reset_on_show = self._reset_on_first_show def _on_show_restart_timer(self): """Callback for '_restart_timer' timer.""" @@ -342,9 +346,13 @@ class PublisherWindow(QtWidgets.QDialog): self.reset() def closeEvent(self, event): - self._controller.save_changes() + self.save_changes() + self._reset_on_show = True super(PublisherWindow, self).closeEvent(event) + def save_changes(self): + self._controller.save_changes() + def reset(self): self._controller.reset() @@ -436,7 +444,8 @@ class PublisherWindow(QtWidgets.QDialog): self._update_publish_frame_rect() def _on_reset_clicked(self): - self._controller.reset() + self.save_changes() + self.reset() def _on_stop_clicked(self): self._controller.stop_publish() From 2d92aed06e530e581229dc96c73ce562ba15ab70 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 15:08:36 +0200 Subject: [PATCH 03/23] handle failed collect,update and remove instances --- openpype/tools/publisher/control.py | 41 ++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/openpype/tools/publisher/control.py b/openpype/tools/publisher/control.py index 1a15a71040..2346825734 100644 --- a/openpype/tools/publisher/control.py +++ b/openpype/tools/publisher/control.py @@ -31,6 +31,9 @@ from openpype.pipeline.create import ( HiddenCreator, Creator, ) +from openpype.pipeline.create.context import ( + CreatorsOperationFailed, +) # Define constant for plugin orders offset PLUGIN_ORDER_OFFSET = 0.5 @@ -1708,8 +1711,18 @@ class PublisherController(BasePublisherController): self._create_context.reset_context_data() with self._create_context.bulk_instances_collection(): - self._create_context.reset_instances() - self._create_context.execute_autocreators() + try: + self._create_context.reset_instances() + self._create_context.execute_autocreators() + + except CreatorsOperationFailed as exc: + self._emit_event( + "instances.collection.failed", + { + "title": "Instance collection failed", + "message": str(exc) + } + ) self._resetting_instances = False @@ -1845,8 +1858,19 @@ class PublisherController(BasePublisherController): def save_changes(self): """Save changes happened during creation.""" - if self._create_context.host_is_valid: + if not self._create_context.host_is_valid: + return + + try: self._create_context.save_changes() + except CreatorsOperationFailed as exc: + self._emit_event( + "instances.save.failed", + { + "title": "Save failed", + "message": str(exc) + } + ) def remove_instances(self, instance_ids): """Remove instances based on instance ids. @@ -1869,7 +1893,16 @@ class PublisherController(BasePublisherController): instances_by_id[instance_id] for instance_id in instance_ids ] - self._create_context.remove_instances(instances) + try: + self._create_context.remove_instances(instances) + except CreatorsOperationFailed as exc: + self._emit_event( + "instances.remove.failed", + { + "title": "Remove failed", + "message": str(exc) + } + ) def _on_create_instance_change(self): self._emit_event("instances.refresh.finished") From f22e3c3a99697f312d35b769a25126effab5c65d Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 15:08:44 +0200 Subject: [PATCH 04/23] show dialogs when creator fails --- openpype/tools/publisher/window.py | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 7559b4a641..a7f2ec2ce6 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -1,3 +1,4 @@ +import collections from Qt import QtWidgets, QtCore, QtGui from openpype import ( @@ -222,6 +223,10 @@ class PublisherWindow(QtWidgets.QDialog): # Floating publish frame publish_frame = PublishFrame(controller, self.footer_border, self) + dialog_message_timer = QtCore.QTimer() + dialog_message_timer.setInterval(100) + dialog_message_timer.timeout.connect(self._on_dialog_message_timeout) + help_btn.clicked.connect(self._on_help_click) tabs_widget.tab_changed.connect(self._on_tab_change) overview_widget.active_changed.connect( @@ -259,6 +264,15 @@ class PublisherWindow(QtWidgets.QDialog): controller.event_system.add_callback( "show.card.message", self._on_overlay_message ) + controller.event_system.add_callback( + "instances.collection.failed", self._instance_collection_failed + ) + controller.event_system.add_callback( + "instances.save.failed", self._instance_save_failed + ) + controller.event_system.add_callback( + "instances.remove.failed", self._instance_remove_failed + ) # Store extra header widget for TrayPublisher # - can be used to add additional widgets to header between context @@ -303,6 +317,9 @@ class PublisherWindow(QtWidgets.QDialog): self._restart_timer = None self._publish_frame_visible = None + self._dialog_messages_to_show = collections.deque() + self._dialog_message_timer = dialog_message_timer + self._set_publish_visibility(False) @property @@ -578,3 +595,51 @@ class PublisherWindow(QtWidgets.QDialog): self._publish_frame.move( 0, window_size.height() - height ) + + def add_message_dialog(self, message, title): + self._dialog_messages_to_show.append((message, title)) + self._dialog_message_timer.start() + + def _on_dialog_message_timeout(self): + if not self._dialog_messages_to_show: + self._dialog_message_timer.stop() + return + + item = self._dialog_messages_to_show.popleft() + message, title = item + dialog = MessageDialog(message, title) + dialog.exec_() + + def _instance_collection_failed(self, event): + self.add_message_dialog(event["message"], event["title"]) + + def _instance_save_failed(self, event): + self.add_message_dialog(event["message"], event["title"]) + + def _instance_remove_failed(self, event): + self.add_message_dialog(event["message"], event["title"]) + + +class MessageDialog(QtWidgets.QDialog): + def __init__(self, message, title, parent=None): + super(MessageDialog, self).__init__(parent) + + self.setWindowTitle(title or "Something happend") + + message_widget = QtWidgets.QLabel(message, self) + + btns_widget = QtWidgets.QWidget(self) + submit_btn = QtWidgets.QPushButton("OK", btns_widget) + + btns_layout = QtWidgets.QHBoxLayout(btns_widget) + btns_layout.setContentsMargins(0, 0, 0, 0) + btns_layout.addStretch(1) + btns_layout.addWidget(submit_btn) + + layout = QtWidgets.QVBoxLayout(self) + layout.addWidget(message_widget, 1) + layout.addWidget(btns_widget, 0) + + def showEvent(self, event): + super(MessageDialog, self).showEvent(event) + self.resize(400, 300) From 34dcbcbf1ec987e77e7390071207b36c36aab284 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 15:17:05 +0200 Subject: [PATCH 05/23] renamed 'CreatorOperationFailed' to 'CreatorsOperationFailed' --- openpype/pipeline/create/context.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openpype/pipeline/create/context.py b/openpype/pipeline/create/context.py index 6bc70f33ea..fb53b95a92 100644 --- a/openpype/pipeline/create/context.py +++ b/openpype/pipeline/create/context.py @@ -62,19 +62,19 @@ class HostMissRequiredMethod(Exception): super(HostMissRequiredMethod, self).__init__(msg) -class CreatorOperationFailed(Exception): +class CreatorsOperationFailed(Exception): pass -class CreatorsCollectionFailed(CreatorOperationFailed): +class CreatorsCollectionFailed(CreatorsOperationFailed): pass -class CreatorsSaveFailed(CreatorOperationFailed): +class CreatorsSaveFailed(CreatorsOperationFailed): pass -class CreatorsRemoveFailed(CreatorOperationFailed): +class CreatorsRemoveFailed(CreatorsOperationFailed): pass From 65ec73a53a8589e4d3ddff7df2d579e4236cf823 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 15:57:21 +0200 Subject: [PATCH 06/23] fix page change --- openpype/tools/publisher/window.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index a7f2ec2ce6..2199981519 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -498,7 +498,7 @@ class PublisherWindow(QtWidgets.QDialog): self._update_publish_details_widget() if ( not self._tabs_widget.is_current_tab("create") - or not self._tabs_widget.is_current_tab("publish") + and not self._tabs_widget.is_current_tab("publish") ): self._tabs_widget.set_current_tab("publish") From d04231cc6b112457b5e440b3ef6bd7e5f0bd475d Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 15:57:30 +0200 Subject: [PATCH 07/23] fix context label before publishing start --- openpype/tools/publisher/control.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openpype/tools/publisher/control.py b/openpype/tools/publisher/control.py index 2346825734..d402ab2434 100644 --- a/openpype/tools/publisher/control.py +++ b/openpype/tools/publisher/control.py @@ -302,8 +302,11 @@ class PublishReport: } def _extract_context_data(self, context): + context_label = "Context" + if context is not None: + context_label = context.data.get("label") return { - "label": context.data.get("label") + "label": context_label } def _extract_instance_data(self, instance, exists): From ab40ab6201a32142b702f7ba1ad1545631c34171 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 16:19:02 +0200 Subject: [PATCH 08/23] change init arg back --- openpype/tools/publisher/window.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 2199981519..0e514bd2f2 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -37,7 +37,7 @@ class PublisherWindow(QtWidgets.QDialog): footer_border = 8 publish_footer_spacer = 2 - def __init__(self, parent=None, controller=None, reset_on_first_show=None): + def __init__(self, parent=None, controller=None, reset_on_show=None): super(PublisherWindow, self).__init__(parent) self.setWindowTitle("OpenPype publisher") @@ -45,8 +45,8 @@ class PublisherWindow(QtWidgets.QDialog): icon = QtGui.QIcon(resources.get_openpype_icon_filepath()) self.setWindowIcon(icon) - if reset_on_first_show is None: - reset_on_first_show = True + if reset_on_show is None: + reset_on_show = True if parent is None: on_top_flag = QtCore.Qt.WindowStaysOnTopHint @@ -312,7 +312,9 @@ class PublisherWindow(QtWidgets.QDialog): self._controller = controller self._first_show = True - self._reset_on_first_show = reset_on_first_show + # This is a little bit confusing but 'reset_on_first_show' is too long + # forin init + self._reset_on_first_show = reset_on_show self._reset_on_show = True self._restart_timer = None self._publish_frame_visible = None From 2e9572aaebfac811a69a2a13b2eb2af8e094c097 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 17:21:43 +0200 Subject: [PATCH 09/23] use float numbers for animation --- openpype/tools/publisher/widgets/overview_widget.py | 7 ++++--- openpype/tools/publisher/widgets/publish_frame.py | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/openpype/tools/publisher/widgets/overview_widget.py b/openpype/tools/publisher/widgets/overview_widget.py index 5bd3017c2a..4cf8ae0eed 100644 --- a/openpype/tools/publisher/widgets/overview_widget.py +++ b/openpype/tools/publisher/widgets/overview_widget.py @@ -93,8 +93,8 @@ class OverviewWidget(QtWidgets.QFrame): main_layout.addWidget(subset_content_widget, 1) change_anim = QtCore.QVariantAnimation() - change_anim.setStartValue(0) - change_anim.setEndValue(self.anim_end_value) + change_anim.setStartValue(float(0)) + change_anim.setEndValue(float(self.anim_end_value)) change_anim.setDuration(self.anim_duration) change_anim.setEasingCurve(QtCore.QEasingCurve.InOutQuad) @@ -264,9 +264,10 @@ class OverviewWidget(QtWidgets.QFrame): + (self._subset_content_layout.spacing() * 2) ) ) - subset_attrs_width = int(float(width) / self.anim_end_value) * value + subset_attrs_width = int((float(width) / self.anim_end_value) * value) if subset_attrs_width > width: subset_attrs_width = width + create_width = width - subset_attrs_width self._create_widget.setMinimumWidth(create_width) diff --git a/openpype/tools/publisher/widgets/publish_frame.py b/openpype/tools/publisher/widgets/publish_frame.py index e6333a104f..00597451a9 100644 --- a/openpype/tools/publisher/widgets/publish_frame.py +++ b/openpype/tools/publisher/widgets/publish_frame.py @@ -248,13 +248,13 @@ class PublishFrame(QtWidgets.QWidget): hint = self._top_content_widget.minimumSizeHint() end = hint.height() - self._shrunk_anim.setStartValue(start) - self._shrunk_anim.setEndValue(end) + self._shrunk_anim.setStartValue(float(start)) + self._shrunk_anim.setEndValue(float(end)) if not anim_is_running: self._shrunk_anim.start() def _on_shrunk_anim(self, value): - diff = self._top_content_widget.height() - value + diff = self._top_content_widget.height() - int(value) if not self._top_content_widget.isVisible(): diff -= self._content_layout.spacing() From 8049bda095ca1290443e1ebcabb1d9a9a54caeca Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Thu, 20 Oct 2022 18:38:00 +0200 Subject: [PATCH 10/23] fix message box --- openpype/tools/publisher/window.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 0e514bd2f2..4f0b81fa85 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -609,8 +609,9 @@ class PublisherWindow(QtWidgets.QDialog): item = self._dialog_messages_to_show.popleft() message, title = item - dialog = MessageDialog(message, title) + dialog = MessageDialog(message, title, self) dialog.exec_() + dialog.deleteLater() def _instance_collection_failed(self, event): self.add_message_dialog(event["message"], event["title"]) @@ -629,6 +630,7 @@ class MessageDialog(QtWidgets.QDialog): self.setWindowTitle(title or "Something happend") message_widget = QtWidgets.QLabel(message, self) + message_widget.setWordWrap(True) btns_widget = QtWidgets.QWidget(self) submit_btn = QtWidgets.QPushButton("OK", btns_widget) @@ -639,9 +641,15 @@ class MessageDialog(QtWidgets.QDialog): btns_layout.addWidget(submit_btn) layout = QtWidgets.QVBoxLayout(self) - layout.addWidget(message_widget, 1) + layout.addWidget(message_widget, 0) + layout.addStretch(1) layout.addWidget(btns_widget, 0) + submit_btn.clicked.connect(self._on_submit_click) + + def _on_submit_click(self): + self.close() + def showEvent(self, event): super(MessageDialog, self).showEvent(event) - self.resize(400, 300) + self.resize(400, 200) From 8e3b2ab933c11ef37678da0b784b971c861afdf6 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:26:39 +0200 Subject: [PATCH 11/23] raise specific exceptions when creators fail to run their methods --- openpype/pipeline/create/context.py | 258 ++++++++++++++++++++++------ 1 file changed, 204 insertions(+), 54 deletions(-) diff --git a/openpype/pipeline/create/context.py b/openpype/pipeline/create/context.py index fb53b95a92..dfa9049601 100644 --- a/openpype/pipeline/create/context.py +++ b/openpype/pipeline/create/context.py @@ -1,6 +1,8 @@ import os +import sys import copy import logging +import traceback import collections import inspect from uuid import uuid4 @@ -22,6 +24,7 @@ from .creator_plugins import ( Creator, AutoCreator, discover_creator_plugins, + CreatorError, ) UpdateData = collections.namedtuple("UpdateData", ["instance", "changes"]) @@ -63,19 +66,76 @@ class HostMissRequiredMethod(Exception): class CreatorsOperationFailed(Exception): - pass + """Raised when a creator process crashes in 'CreateContext'. + + The exception contains information about the creator and error. The data + are prepared using 'prepare_failed_creator_operation_info' and can be + serialized using json. + + Usage is for UI purposes which may not have access to exceptions directly + and would not have ability to catch exceptions 'per creator'. + + Args: + msg (str): General error message. + failed_info (list[dict[str, Any]]): List of failed creators with + exception message and optionally formatted traceback. + """ + + def __init__(self, msg, failed_info): + super(CreatorsOperationFailed, self).__init__(msg) + self.failed_info = failed_info class CreatorsCollectionFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed to collect instances" + super(CreatorsCollectionFailed, self).__init__( + msg, failed_info + ) class CreatorsSaveFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed update instance changes" + super(CreatorsSaveFailed, self).__init__( + msg, failed_info + ) class CreatorsRemoveFailed(CreatorsOperationFailed): - pass + def __init__(self, failed_info): + msg = "Failed to remove instances" + super(CreatorsRemoveFailed, self).__init__( + msg, failed_info + ) + + +class CreatorsCreateFailed(CreatorsOperationFailed): + def __init__(self, failed_info): + msg = "Faled to create instances" + super(CreatorsCreateFailed, self).__init__( + msg, failed_info + ) + + +def prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback=True +): + formatted_traceback = None + exc_type, exc_value, exc_traceback = exc_info + error_msg = str(exc_value) + + if add_traceback: + formatted_traceback = "".join(traceback.format_exception( + exc_type, exc_value, exc_traceback + )) + + return { + "creator_identifier": identifier, + "creator_label": label, + "message": error_msg, + "traceback": formatted_traceback + } class InstanceMember: @@ -1202,7 +1262,67 @@ class CreateContext: with self.bulk_instances_collection(): self._bulk_instances_to_process.append(instance) + def create(self, identifier, *args, **kwargs): + """Wrapper for creators to trigger created. + + Different types of creators may expect different arguments thus the + hints for args are blind. + + Args: + identifier (str): Creator's identifier. + *args (Tuple[Any]): Arguments for create method. + **kwargs (Dict[Any, Any]): Keyword argument for create method. + """ + + creator = self.creators.get(identifier) + label = getattr(creator, "label", None) + failed = False + add_traceback = False + try: + # Fake CreatorError (Could be maybe specific exception?) + if creator is None: + raise CreatorError( + "Creator {} was not found".format(identifier) + ) + + creator.create(*args, **kwargs) + + except CreatorError: + failed = True + exc_info = sys.exc_info() + + except: + failed = True + add_traceback = True + exc_info = sys.exc_info() + + if not failed: + return + + self.log.warning( + ( + "Failed to run Creator with identifier \"{}\"." + ).format(identifier), + exc_info=add_traceback + ) + + raise CreatorsCreateFailed([ + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ]) + def creator_removed_instance(self, instance): + """When creator removes instance context should be acknowledged. + + If creator removes instance conext should know about it to avoid + possible issues in the session. + + Args: + instance (CreatedInstance): Object of instance which was removed + from scene metadata. + """ + self._instances_by_id.pop(instance.id, None) @contextmanager @@ -1237,42 +1357,72 @@ class CreateContext: self._instances_by_id = {} # Collect instances - failed_creators = [] + failed_info = [] for creator in self.creators.values(): + label = creator.label try: creator.collect_instances() except: - failed_creators.append(creator) + exc_info = sys.exc_info() + identifier = creator.identifier self.log.warning( - "Collection of instances for creator {} ({}) failed".format( - creator.label, creator.identifier), + ( + "Collection of instances for creator {} failed" + ).format(identifier), exc_info=True ) + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info + ) + ) - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsCollectionFailed( - "Failed to collect instances of creators {}".format(joined_creators) - ) + if failed_info: + raise CreatorsCollectionFailed(failed_info) def execute_autocreators(self): """Execute discovered AutoCreator plugins. Reset instances if any autocreator executed properly. """ + + failed_info = [] for identifier, creator in self.autocreators.items(): + label = creator.label + failed = False + add_traceback = False try: creator.create() - except Exception: - # TODO raise report exception if any crashed - msg = ( - "Failed to run AutoCreator with identifier \"{}\" ({})." - ).format(identifier, inspect.getfile(creator.__class__)) - self.log.warning(msg, exc_info=True) + except CreatorError: + failed = True + exc_info = sys.exc_info() + + # Use bare except because some hosts raise their exceptions that + # do not inherit from python's `BaseException` + except: + failed = True + add_traceback = True + exc_info = sys.exc_info() + + if not failed: + continue + + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ) + + self.log.warning( + ( + "Failed to run AutoCreator with identifier \"{}\"." + ).format(identifier), + exc_info=exc_info + ) + + if failed_info: + raise CreatorsCreateFailed(failed_info) def validate_instances_context(self, instances=None): """Validate 'asset' and 'task' instance context.""" @@ -1349,7 +1499,7 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) - failed_creators = [] + failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): update_list = [] for instance in creator_instances: @@ -1358,26 +1508,29 @@ class CreateContext: update_list.append(UpdateData(instance, instance_changes)) creator = self.creators[identifier] - if update_list: - try: - creator.update_instances(update_list) + if not update_list: + continue - except: - failed_creators.append(creator) - self.log.warning( - "Instances update of creator {} ({}) failed".format( - creator.label, creator.identifier), - exc_info=True + label = creator.label + try: + creator.update_instances(update_list) + + except: + exc_info = sys.exc_info() + self.log.warning( + "Instances update of creator \"{}\" failed".format( + identifier), + exc_info=True + ) + + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info ) + ) - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsSaveFailed( - "Failed save changes of creators {}".format(joined_creators) - ) + if failed_info: + raise CreatorsSaveFailed(failed_info) def remove_instances(self, instances): """Remove instances from context. @@ -1392,30 +1545,27 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) - failed_creators = [] + failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): creator = self.creators.get(identifier) + label = creator.label try: creator.remove_instances(creator_instances) except: - failed_creators.append(creator) + exc_info = sys.exc_info() self.log.warning( - "Instances removement of creator {} ({}) failed".format( - creator.label, creator.identifier), + "Instances removement of creator \"{}\" failed".format( + identifier), exc_info=True ) - - if failed_creators: - joined_creators = ", ".join( - [creator.label for creator in failed_creators] - ) - - raise CreatorsRemoveFailed( - "Failed to remove instances of creators {}".format( - joined_creators + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info + ) ) - ) + if failed_info: + raise CreatorsRemoveFailed(failed_info) def _get_publish_plugins_with_attr_for_family(self, family): """Publish plugin attributes for passed family. From 91fa300d997abe7f483965c1d342949d6460e61e Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:57:45 +0200 Subject: [PATCH 12/23] implementaed separator widget --- openpype/tools/utils/widgets.py | 54 +++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/openpype/tools/utils/widgets.py b/openpype/tools/utils/widgets.py index c8133b3359..ca65182124 100644 --- a/openpype/tools/utils/widgets.py +++ b/openpype/tools/utils/widgets.py @@ -448,3 +448,57 @@ class OptionDialog(QtWidgets.QDialog): def parse(self): return self._options.copy() + + +class SeparatorWidget(QtWidgets.QFrame): + """Prepared widget that can be used as separator with predefined color. + + Args: + size (int): Size of separator (width or height). + orientation (Qt.Horizontal|Qt.Vertical): Orintation of widget. + parent (QtWidgets.QWidget): Parent widget. + """ + + def __init__(self, size=2, orientation=QtCore.Qt.Horizontal, parent=None): + super(SeparatorWidget, self).__init__(parent) + + self.setObjectName("Separator") + + maximum_width = self.maximumWidth() + maximum_height = self.maximumHeight() + + self._size = None + self._orientation = orientation + self._maximum_width = maximum_width + self._maximum_height = maximum_height + self.set_size(size) + + def set_size(self, size): + if size == self._size: + return + if self._orientation == QtCore.Qt.Vertical: + self.setMinimumWidth(size) + self.setMaximumWidth(size) + else: + self.setMinimumHeight(size) + self.setMaximumHeight(size) + + self._size = size + + def set_orientation(self, orientation): + if self._orientation == orientation: + return + + # Reset min/max sizes in opossite direction + if self._orientation == QtCore.Qt.Vertical: + self.setMinimumHeight(0) + self.setMaximumHeight(self._maximum_height) + else: + self.setMinimumWidth(0) + self.setMaximumWidth(self._maximum_width) + + self._orientation = orientation + + size = self._size + self._size = None + self.set_size(size) From 20dd53a830b6745a89b4203c681e8eea54b079cc Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:58:08 +0200 Subject: [PATCH 13/23] use separator widget in error dialog --- openpype/tools/utils/error_dialog.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/openpype/tools/utils/error_dialog.py b/openpype/tools/utils/error_dialog.py index f7b12bb69f..30cba56416 100644 --- a/openpype/tools/utils/error_dialog.py +++ b/openpype/tools/utils/error_dialog.py @@ -1,6 +1,6 @@ from Qt import QtWidgets, QtCore -from .widgets import ClickableFrame, ExpandBtn +from .widgets import ClickableFrame, ExpandBtn, SeparatorWidget def convert_text_for_html(text): @@ -139,12 +139,10 @@ class ErrorMessageBox(QtWidgets.QDialog): mime_data ) - def _create_line(self): - line = QtWidgets.QFrame(self) - line.setObjectName("Separator") - line.setMinimumHeight(2) - line.setMaximumHeight(2) - return line + def _create_line(self, parent=None): + if parent is None: + parent = self + return SeparatorWidget(2, parent=parent) def _create_traceback_widget(self, traceback_text, parent=None): if parent is None: From 9ae99eb4badd874b1181697179fc9747963a2c58 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:58:34 +0200 Subject: [PATCH 14/23] change function name 'convert_text_for_html' to 'escape_text_for_html' --- openpype/tools/utils/error_dialog.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openpype/tools/utils/error_dialog.py b/openpype/tools/utils/error_dialog.py index 30cba56416..d4ca91848c 100644 --- a/openpype/tools/utils/error_dialog.py +++ b/openpype/tools/utils/error_dialog.py @@ -3,7 +3,7 @@ from Qt import QtWidgets, QtCore from .widgets import ClickableFrame, ExpandBtn, SeparatorWidget -def convert_text_for_html(text): +def escape_text_for_html(text): return ( text .replace("<", "<") @@ -19,7 +19,7 @@ class TracebackWidget(QtWidgets.QWidget): # Modify text to match html # - add more replacements when needed - tb_text = convert_text_for_html(tb_text) + tb_text = escape_text_for_html(tb_text) expand_btn = ExpandBtn(self) clickable_frame = ClickableFrame(self) @@ -110,7 +110,7 @@ class ErrorMessageBox(QtWidgets.QDialog): @staticmethod def convert_text_for_html(text): - return convert_text_for_html(text) + return escape_text_for_html(text) def _create_top_widget(self, parent_widget): label_widget = QtWidgets.QLabel(parent_widget) From ad28ea8121dc62f7dbe708ef3f2bbbe21d111810 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 10:58:55 +0200 Subject: [PATCH 15/23] top widget is optional --- openpype/tools/utils/error_dialog.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openpype/tools/utils/error_dialog.py b/openpype/tools/utils/error_dialog.py index d4ca91848c..9c9015c00f 100644 --- a/openpype/tools/utils/error_dialog.py +++ b/openpype/tools/utils/error_dialog.py @@ -91,11 +91,12 @@ class ErrorMessageBox(QtWidgets.QDialog): footer_layout.addWidget(ok_btn, 0) bottom_line = self._create_line() - body_layout = QtWidgets.QVBoxLayout(self) - body_layout.addWidget(top_widget, 0) - body_layout.addWidget(content_scroll, 1) - body_layout.addWidget(bottom_line, 0) - body_layout.addLayout(footer_layout, 0) + main_layout = QtWidgets.QVBoxLayout(self) + if top_widget is not None: + main_layout.addWidget(top_widget, 0) + main_layout.addWidget(content_scroll, 1) + main_layout.addWidget(bottom_line, 0) + main_layout.addWidget(footer_widget, 0) copy_report_btn.clicked.connect(self._on_copy_report) ok_btn.clicked.connect(self._on_ok_clicked) From 280e4fe0ca3d32ae506969a249034bfd720976f5 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:00:59 +0200 Subject: [PATCH 16/23] change separator of report copy --- openpype/tools/utils/error_dialog.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openpype/tools/utils/error_dialog.py b/openpype/tools/utils/error_dialog.py index 9c9015c00f..6973fda8c4 100644 --- a/openpype/tools/utils/error_dialog.py +++ b/openpype/tools/utils/error_dialog.py @@ -132,7 +132,8 @@ class ErrorMessageBox(QtWidgets.QDialog): self.close() def _on_copy_report(self): - report_text = (10 * "*").join(self._report_data) + sep = "\n{}\n".format(10 * "*") + report_text = sep.join(self._report_data) mime_data = QtCore.QMimeData() mime_data.setText(report_text) From 239d2f90bfd729f1a6fe58883b6fd992e9242de2 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:01:41 +0200 Subject: [PATCH 17/23] footer layout is under widget and store the widget to self --- openpype/tools/utils/error_dialog.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/openpype/tools/utils/error_dialog.py b/openpype/tools/utils/error_dialog.py index 6973fda8c4..5fe49a53af 100644 --- a/openpype/tools/utils/error_dialog.py +++ b/openpype/tools/utils/error_dialog.py @@ -85,7 +85,9 @@ class ErrorMessageBox(QtWidgets.QDialog): copy_report_btn = QtWidgets.QPushButton("Copy report", self) ok_btn = QtWidgets.QPushButton("OK", self) - footer_layout = QtWidgets.QHBoxLayout() + footer_widget = QtWidgets.QWidget(self) + footer_layout = QtWidgets.QHBoxLayout(footer_widget) + footer_layout.setContentsMargins(0, 0, 0, 0) footer_layout.addWidget(copy_report_btn, 0) footer_layout.addStretch(1) footer_layout.addWidget(ok_btn, 0) @@ -107,6 +109,8 @@ class ErrorMessageBox(QtWidgets.QDialog): if not report_data: copy_report_btn.setVisible(False) + self._content_scroll = content_scroll + self._footer_widget = footer_widget self._report_data = report_data @staticmethod From f4b123d65a5a10e871dfdd2adf1fe6c92a2a6727 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:01:52 +0200 Subject: [PATCH 18/23] added separator widget to public widgets --- openpype/tools/utils/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openpype/tools/utils/__init__.py b/openpype/tools/utils/__init__.py index 5ccc1b40b3..019ea16391 100644 --- a/openpype/tools/utils/__init__.py +++ b/openpype/tools/utils/__init__.py @@ -7,6 +7,7 @@ from .widgets import ( ExpandBtn, PixmapLabel, IconButton, + SeparatorWidget, ) from .views import DeselectableTreeView from .error_dialog import ErrorMessageBox @@ -37,6 +38,7 @@ __all__ = ( "ExpandBtn", "PixmapLabel", "IconButton", + "SeparatorWidget", "DeselectableTreeView", From 36a9aa2da61d583e9ddd8209b92614f751a42bbf Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:02:57 +0200 Subject: [PATCH 19/23] modified error dialog to show more information by creator --- openpype/tools/publisher/window.py | 153 +++++++++++++++++++++-------- 1 file changed, 114 insertions(+), 39 deletions(-) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 4f0b81fa85..1bbc0d0cf4 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -6,6 +6,7 @@ from openpype import ( style ) from openpype.tools.utils import ( + ErrorMessageBox, PlaceholderLineEdit, MessageOverlayObject, PixmapLabel, @@ -223,9 +224,11 @@ class PublisherWindow(QtWidgets.QDialog): # Floating publish frame publish_frame = PublishFrame(controller, self.footer_border, self) - dialog_message_timer = QtCore.QTimer() - dialog_message_timer.setInterval(100) - dialog_message_timer.timeout.connect(self._on_dialog_message_timeout) + creators_dialog_message_timer = QtCore.QTimer() + creators_dialog_message_timer.setInterval(100) + creators_dialog_message_timer.timeout.connect( + self._on_creators_message_timeout + ) help_btn.clicked.connect(self._on_help_click) tabs_widget.tab_changed.connect(self._on_tab_change) @@ -273,6 +276,9 @@ class PublisherWindow(QtWidgets.QDialog): controller.event_system.add_callback( "instances.remove.failed", self._instance_remove_failed ) + controller.event_system.add_callback( + "instances.create.failed", self._instance_create_failed + ) # Store extra header widget for TrayPublisher # - can be used to add additional widgets to header between context @@ -319,8 +325,8 @@ class PublisherWindow(QtWidgets.QDialog): self._restart_timer = None self._publish_frame_visible = None - self._dialog_messages_to_show = collections.deque() - self._dialog_message_timer = dialog_message_timer + self._creators_messages_to_show = collections.deque() + self._creators_dialog_message_timer = creators_dialog_message_timer self._set_publish_visibility(False) @@ -598,58 +604,127 @@ class PublisherWindow(QtWidgets.QDialog): 0, window_size.height() - height ) - def add_message_dialog(self, message, title): - self._dialog_messages_to_show.append((message, title)) - self._dialog_message_timer.start() + def add_message_dialog(self, title, failed_info): + self._creators_messages_to_show.append((title, failed_info)) + self._creators_dialog_message_timer.start() - def _on_dialog_message_timeout(self): - if not self._dialog_messages_to_show: - self._dialog_message_timer.stop() + def _on_creators_message_timeout(self): + if not self._creators_messages_to_show: + self._creators_dialog_message_timer.stop() return - item = self._dialog_messages_to_show.popleft() - message, title = item - dialog = MessageDialog(message, title, self) + item = self._creators_messages_to_show.popleft() + title, failed_info = item + dialog = CreatorsErrorMessageBox(title, failed_info, self) dialog.exec_() dialog.deleteLater() def _instance_collection_failed(self, event): - self.add_message_dialog(event["message"], event["title"]) + self.add_message_dialog(event["title"], event["failed_info"]) def _instance_save_failed(self, event): - self.add_message_dialog(event["message"], event["title"]) + self.add_message_dialog(event["title"], event["failed_info"]) def _instance_remove_failed(self, event): - self.add_message_dialog(event["message"], event["title"]) + self.add_message_dialog(event["title"], event["failed_info"]) + + def _instance_create_failed(self, event): + self.add_message_dialog(event["title"], event["failed_info"]) -class MessageDialog(QtWidgets.QDialog): - def __init__(self, message, title, parent=None): - super(MessageDialog, self).__init__(parent) +class CreatorsErrorMessageBox(ErrorMessageBox): + def __init__(self, error_title, failed_info, parent): + self._failed_info = failed_info + self._info_with_id = [ + {"id": idx, "info": info} + for idx, info in enumerate(failed_info) + ] + self._widgets_by_id = {} + self._tabs_widget = None + self._stack_layout = None - self.setWindowTitle(title or "Something happend") + super(CreatorsErrorMessageBox, self).__init__(error_title, parent) - message_widget = QtWidgets.QLabel(message, self) - message_widget.setWordWrap(True) + layout = self.layout() + layout.setContentsMargins(0, 0, 0, 0) + layout.setSpacing(0) - btns_widget = QtWidgets.QWidget(self) - submit_btn = QtWidgets.QPushButton("OK", btns_widget) + footer_layout = self._footer_widget.layout() + footer_layout.setContentsMargins(5, 5, 5, 5) - btns_layout = QtWidgets.QHBoxLayout(btns_widget) - btns_layout.setContentsMargins(0, 0, 0, 0) - btns_layout.addStretch(1) - btns_layout.addWidget(submit_btn) + def _create_top_widget(self, parent_widget): + return None - layout = QtWidgets.QVBoxLayout(self) - layout.addWidget(message_widget, 0) - layout.addStretch(1) - layout.addWidget(btns_widget, 0) + def _get_report_data(self): + output = [] + for info in self._failed_info: + creator_label = info["creator_label"] + creator_identifier = info["creator_identifier"] + report_message = "Creator:" + if creator_label: + report_message += " {} ({})".format( + creator_label, creator_identifier) + else: + report_message += " {}".format(creator_identifier) - submit_btn.clicked.connect(self._on_submit_click) + report_message += "\n\nError: {}".format(info["message"]) + formatted_traceback = info["traceback"] + if formatted_traceback: + report_message += "\n\n{}".format(formatted_traceback) + output.append(report_message) + return output - def _on_submit_click(self): - self.close() + def _create_content(self, content_layout): + tabs_widget = PublisherTabsWidget(self) - def showEvent(self, event): - super(MessageDialog, self).showEvent(event) - self.resize(400, 200) + stack_widget = QtWidgets.QFrame(self._content_widget) + stack_layout = QtWidgets.QStackedLayout(stack_widget) + + first = True + for item in self._info_with_id: + item_id = item["id"] + info = item["info"] + message = info["message"] + formatted_traceback = info["traceback"] + creator_label = info["creator_label"] + creator_identifier = info["creator_identifier"] + if not creator_label: + creator_label = creator_identifier + + msg_widget = QtWidgets.QWidget(stack_widget) + msg_layout = QtWidgets.QVBoxLayout(msg_widget) + + exc_msg_template = "{}" + message_label_widget = QtWidgets.QLabel(msg_widget) + message_label_widget.setText( + exc_msg_template.format(self.convert_text_for_html(message)) + ) + msg_layout.addWidget(message_label_widget, 0) + + if formatted_traceback: + line_widget = self._create_line(msg_widget) + tb_widget = self._create_traceback_widget(formatted_traceback) + msg_layout.addWidget(line_widget, 0) + msg_layout.addWidget(tb_widget, 0) + + msg_layout.addStretch(1) + + tabs_widget.add_tab(creator_label, item_id) + stack_layout.addWidget(msg_widget) + if first: + first = False + stack_layout.setCurrentWidget(msg_widget) + + self._widgets_by_id[item_id] = msg_widget + + content_layout.addWidget(tabs_widget, 0) + content_layout.addWidget(stack_widget, 1) + + tabs_widget.tab_changed.connect(self._on_tab_change) + + self._tabs_widget = tabs_widget + self._stack_layout = stack_layout + + def _on_tab_change(self, identifier): + widget = self._widgets_by_id[identifier] + self._stack_layout.setCurrentWidget(widget) From 098903260c79be64c58a07eb7616e24c04c41de9 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:03:15 +0200 Subject: [PATCH 20/23] controller is triggering event on creator operation fail --- openpype/tools/publisher/control.py | 45 +++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/openpype/tools/publisher/control.py b/openpype/tools/publisher/control.py index d402ab2434..280d2cf0a0 100644 --- a/openpype/tools/publisher/control.py +++ b/openpype/tools/publisher/control.py @@ -1107,6 +1107,8 @@ class AbstractPublisherController(object): options (Dict[str, Any]): Data from pre-create attributes. """ + pass + def save_changes(self): """Save changes in create context.""" @@ -1716,14 +1718,24 @@ class PublisherController(BasePublisherController): with self._create_context.bulk_instances_collection(): try: self._create_context.reset_instances() - self._create_context.execute_autocreators() - except CreatorsOperationFailed as exc: self._emit_event( "instances.collection.failed", { "title": "Instance collection failed", - "message": str(exc) + "failed_info": exc.failed_info + } + ) + + try: + self._create_context.execute_autocreators() + + except CreatorsOperationFailed as exc: + self._emit_event( + "instances.create.failed", + { + "title": "AutoCreation failed", + "failed_info": exc.failed_info } ) @@ -1854,10 +1866,24 @@ class PublisherController(BasePublisherController): self, creator_identifier, subset_name, instance_data, options ): """Trigger creation and refresh of instances in UI.""" - creator = self._creators[creator_identifier] - creator.create(subset_name, instance_data, options) + + success = True + try: + self._create_context.create( + creator_identifier, subset_name, instance_data, options + ) + except CreatorsOperationFailed as exc: + success = False + self._emit_event( + "instances.create.failed", + { + "title": "Creation failed", + "failed_info": exc.failed_info + } + ) self._on_create_instance_change() + return success def save_changes(self): """Save changes happened during creation.""" @@ -1866,12 +1892,13 @@ class PublisherController(BasePublisherController): try: self._create_context.save_changes() + except CreatorsOperationFailed as exc: self._emit_event( "instances.save.failed", { - "title": "Save failed", - "message": str(exc) + "title": "Instances save failed", + "failed_info": exc.failed_info } ) @@ -1902,8 +1929,8 @@ class PublisherController(BasePublisherController): self._emit_event( "instances.remove.failed", { - "title": "Remove failed", - "message": str(exc) + "title": "Instance removement failed", + "failed_info": exc.failed_info } ) From accab0ca5f17996ff0db8e178dae1782eec31c4a Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:03:30 +0200 Subject: [PATCH 21/23] create widget does not handle failed creation on it's own --- .../tools/publisher/widgets/create_widget.py | 118 ++---------------- 1 file changed, 7 insertions(+), 111 deletions(-) diff --git a/openpype/tools/publisher/widgets/create_widget.py b/openpype/tools/publisher/widgets/create_widget.py index 10cf39675e..910b2adfc7 100644 --- a/openpype/tools/publisher/widgets/create_widget.py +++ b/openpype/tools/publisher/widgets/create_widget.py @@ -9,7 +9,6 @@ from openpype.pipeline.create import ( SUBSET_NAME_ALLOWED_SYMBOLS, TaskNotSetError, ) -from openpype.tools.utils import ErrorMessageBox from .widgets import ( IconValuePixmapLabel, @@ -35,79 +34,6 @@ class VariantInputsWidget(QtWidgets.QWidget): self.resized.emit() -class CreateErrorMessageBox(ErrorMessageBox): - def __init__( - self, - creator_label, - subset_name, - asset_name, - exc_msg, - formatted_traceback, - parent - ): - self._creator_label = creator_label - self._subset_name = subset_name - self._asset_name = asset_name - self._exc_msg = exc_msg - self._formatted_traceback = formatted_traceback - super(CreateErrorMessageBox, self).__init__("Creation failed", parent) - - def _create_top_widget(self, parent_widget): - label_widget = QtWidgets.QLabel(parent_widget) - label_widget.setText( - "Failed to create" - ) - return label_widget - - def _get_report_data(self): - report_message = ( - "{creator}: Failed to create Subset: \"{subset}\"" - " in Asset: \"{asset}\"" - "\n\nError: {message}" - ).format( - creator=self._creator_label, - subset=self._subset_name, - asset=self._asset_name, - message=self._exc_msg, - ) - if self._formatted_traceback: - report_message += "\n\n{}".format(self._formatted_traceback) - return [report_message] - - def _create_content(self, content_layout): - item_name_template = ( - "Creator: {}
" - "Subset: {}
" - "Asset: {}
" - ) - exc_msg_template = "{}" - - line = self._create_line() - content_layout.addWidget(line) - - item_name_widget = QtWidgets.QLabel(self) - item_name_widget.setText( - item_name_template.format( - self._creator_label, self._subset_name, self._asset_name - ) - ) - content_layout.addWidget(item_name_widget) - - message_label_widget = QtWidgets.QLabel(self) - message_label_widget.setText( - exc_msg_template.format(self.convert_text_for_html(self._exc_msg)) - ) - content_layout.addWidget(message_label_widget) - - if self._formatted_traceback: - line_widget = self._create_line() - tb_widget = self._create_traceback_widget( - self._formatted_traceback - ) - content_layout.addWidget(line_widget) - content_layout.addWidget(tb_widget) - - # TODO add creator identifier/label to details class CreatorShortDescWidget(QtWidgets.QWidget): def __init__(self, parent=None): @@ -178,8 +104,6 @@ class CreateWidget(QtWidgets.QWidget): self._prereq_available = False - self._message_dialog = None - name_pattern = "^[{}]*$".format(SUBSET_NAME_ALLOWED_SYMBOLS) self._name_pattern = name_pattern self._compiled_name_pattern = re.compile(name_pattern) @@ -769,7 +693,6 @@ class CreateWidget(QtWidgets.QWidget): return index = indexes[0] - creator_label = index.data(QtCore.Qt.DisplayRole) creator_identifier = index.data(CREATOR_IDENTIFIER_ROLE) family = index.data(FAMILY_ROLE) variant = self.variant_input.text() @@ -792,40 +715,13 @@ class CreateWidget(QtWidgets.QWidget): "family": family } - error_msg = None - formatted_traceback = None - try: - self._controller.create( - creator_identifier, - subset_name, - instance_data, - pre_create_data - ) + success = self._controller.create( + creator_identifier, + subset_name, + instance_data, + pre_create_data + ) - except CreatorError as exc: - error_msg = str(exc) - - # Use bare except because some hosts raise their exceptions that - # do not inherit from python's `BaseException` - except: - exc_type, exc_value, exc_traceback = sys.exc_info() - formatted_traceback = "".join(traceback.format_exception( - exc_type, exc_value, exc_traceback - )) - error_msg = str(exc_value) - - if error_msg is None: + if success: self._set_creator(self._selected_creator) self._controller.emit_card_message("Creation finished...") - else: - box = CreateErrorMessageBox( - creator_label, - subset_name, - asset_name, - error_msg, - formatted_traceback, - parent=self - ) - box.show() - # Store dialog so is not garbage collected before is shown - self._message_dialog = box From bda1bb3f292c05004546870ddd39d4c4df8bd384 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:46:02 +0200 Subject: [PATCH 22/23] unify the error handling in create context --- openpype/pipeline/create/context.py | 123 +++++++++++++++++----------- 1 file changed, 76 insertions(+), 47 deletions(-) diff --git a/openpype/pipeline/create/context.py b/openpype/pipeline/create/context.py index dfa9049601..2dfdfc142f 100644 --- a/openpype/pipeline/create/context.py +++ b/openpype/pipeline/create/context.py @@ -123,8 +123,6 @@ def prepare_failed_creator_operation_info( ): formatted_traceback = None exc_type, exc_value, exc_traceback = exc_info - error_msg = str(exc_value) - if add_traceback: formatted_traceback = "".join(traceback.format_exception( exc_type, exc_value, exc_traceback @@ -133,7 +131,7 @@ def prepare_failed_creator_operation_info( return { "creator_identifier": identifier, "creator_label": label, - "message": error_msg, + "message": str(exc_value), "traceback": formatted_traceback } @@ -1274,10 +1272,12 @@ class CreateContext: **kwargs (Dict[Any, Any]): Keyword argument for create method. """ + error_message = "Failed to run Creator with identifier \"{}\". {}" creator = self.creators.get(identifier) label = getattr(creator, "label", None) failed = False add_traceback = False + exc_info = None try: # Fake CreatorError (Could be maybe specific exception?) if creator is None: @@ -1290,27 +1290,23 @@ class CreateContext: except CreatorError: failed = True exc_info = sys.exc_info() + self.log.warning(error_message.format(identifier, exc_info[1])) except: failed = True add_traceback = True exc_info = sys.exc_info() - - if not failed: - return - - self.log.warning( - ( - "Failed to run Creator with identifier \"{}\"." - ).format(identifier), - exc_info=add_traceback - ) - - raise CreatorsCreateFailed([ - prepare_failed_creator_operation_info( - identifier, label, exc_info, add_traceback + self.log.warning( + error_message.format(identifier, ""), + exc_info=True ) - ]) + + if failed: + raise CreatorsCreateFailed([ + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ]) def creator_removed_instance(self, instance): """When creator removes instance context should be acknowledged. @@ -1357,23 +1353,35 @@ class CreateContext: self._instances_by_id = {} # Collect instances + error_message = "Collection of instances for creator {} failed. {}" failed_info = [] for creator in self.creators.values(): label = creator.label + identifier = creator.identifier + failed = False + add_traceback = False + exc_info = None try: creator.collect_instances() - except: + + except CreatorError: + failed = True + exc_info = sys.exc_info() + self.log.warning(error_message.format(identifier, exc_info[1])) + + except: + failed = True + add_traceback = True exc_info = sys.exc_info() - identifier = creator.identifier self.log.warning( - ( - "Collection of instances for creator {} failed" - ).format(identifier), + error_message.format(identifier, ""), exc_info=True ) + + if failed: failed_info.append( prepare_failed_creator_operation_info( - identifier, label, exc_info + identifier, label, exc_info, add_traceback ) ) @@ -1386,6 +1394,7 @@ class CreateContext: Reset instances if any autocreator executed properly. """ + error_message = "Failed to run AutoCreator with identifier \"{}\". {}" failed_info = [] for identifier, creator in self.autocreators.items(): label = creator.label @@ -1397,6 +1406,7 @@ class CreateContext: except CreatorError: failed = True exc_info = sys.exc_info() + self.log.warning(error_message.format(identifier, exc_info[1])) # Use bare except because some hosts raise their exceptions that # do not inherit from python's `BaseException` @@ -1404,22 +1414,17 @@ class CreateContext: failed = True add_traceback = True exc_info = sys.exc_info() - - if not failed: - continue - - failed_info.append( - prepare_failed_creator_operation_info( - identifier, label, exc_info, add_traceback + self.log.warning( + error_message.format(identifier, ""), + exc_info=True ) - ) - self.log.warning( - ( - "Failed to run AutoCreator with identifier \"{}\"." - ).format(identifier), - exc_info=exc_info - ) + if failed: + failed_info.append( + prepare_failed_creator_operation_info( + identifier, label, exc_info, add_traceback + ) + ) if failed_info: raise CreatorsCreateFailed(failed_info) @@ -1499,6 +1504,7 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) + error_message = "Instances update of creator \"{}\" failed. {}" failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): update_list = [] @@ -1512,20 +1518,28 @@ class CreateContext: continue label = creator.label + failed = False + add_traceback = False + exc_info = None try: creator.update_instances(update_list) + except CreatorError: + failed = True + exc_info = sys.exc_info() + self.log.warning(error_message.format(identifier, exc_info[1])) + except: + failed = True + add_traceback = True exc_info = sys.exc_info() self.log.warning( - "Instances update of creator \"{}\" failed".format( - identifier), - exc_info=True - ) + error_message.format(identifier, ""), exc_info=True) + if failed: failed_info.append( prepare_failed_creator_operation_info( - identifier, label, exc_info + identifier, label, exc_info, add_traceback ) ) @@ -1545,22 +1559,37 @@ class CreateContext: identifier = instance.creator_identifier instances_by_identifier[identifier].append(instance) + error_message = "Instances removement of creator \"{}\" failed. {}" failed_info = [] for identifier, creator_instances in instances_by_identifier.items(): creator = self.creators.get(identifier) label = creator.label + failed = False + add_traceback = False + exc_info = None try: creator.remove_instances(creator_instances) - except: + + except CreatorError: + failed = True exc_info = sys.exc_info() self.log.warning( - "Instances removement of creator \"{}\" failed".format( - identifier), + error_message.format(identifier, exc_info[1]) + ) + + except: + failed = True + add_traceback = True + exc_info = sys.exc_info() + self.log.warning( + error_message.format(identifier, ""), exc_info=True ) + + if failed: failed_info.append( prepare_failed_creator_operation_info( - identifier, label, exc_info + identifier, label, exc_info, add_traceback ) ) From 1f2e54c2c385af56c5d61d2976e8db434d9cb4ec Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Fri, 21 Oct 2022 11:46:21 +0200 Subject: [PATCH 23/23] fix tab switch --- openpype/tools/publisher/window.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openpype/tools/publisher/window.py b/openpype/tools/publisher/window.py index 1bbc0d0cf4..b6bd506c18 100644 --- a/openpype/tools/publisher/window.py +++ b/openpype/tools/publisher/window.py @@ -636,7 +636,8 @@ class CreatorsErrorMessageBox(ErrorMessageBox): def __init__(self, error_title, failed_info, parent): self._failed_info = failed_info self._info_with_id = [ - {"id": idx, "info": info} + # Id must be string when used in tab widget + {"id": str(idx), "info": info} for idx, info in enumerate(failed_info) ] self._widgets_by_id = {} @@ -725,6 +726,6 @@ class CreatorsErrorMessageBox(ErrorMessageBox): self._tabs_widget = tabs_widget self._stack_layout = stack_layout - def _on_tab_change(self, identifier): + def _on_tab_change(self, old_identifier, identifier): widget = self._widgets_by_id[identifier] self._stack_layout.setCurrentWidget(widget)