SHOT-4385: Improvements (#34)

* Improve error handling and display critical errors more visibly

python/tk_multi_data_validation/api/data/validation_rule.py

@@ -131,6 +131,7 @@ def __init__(self, rule_data, bundle=None):
         self._valid = None
         # The error items found the last time the rule's check function was executed.
         self._error_items = None
+        self._error_count = 0
         # Flag indicating that the rule fix method was executed at least once
         self._fix_executed = False
         # Runtime exceptions caught - used to display error messages
@@ -339,6 +340,17 @@ def errors(self):
         """
         return self._error_items
 
+    @property
+    def error_count(self):
+        """
+        Get the number of errors found by the rule's check function the last
+        time it was executed.
+
+        Validation checks may opt to return the error count instead of the error
+        items for performance reasons.
+        """
+        return self._error_count
+
     @property
     def fix_executed(self):
         """Get the flag indicating if the rule's fix method was executed at least once."""
@@ -420,19 +432,59 @@ def get_error_messages(self):
 
         messages = []
 
-        if self._check_runtime_exception:
+        if isinstance(self._check_runtime_exception, ConnectionError) or isinstance(
+            self._fix_runtime_exception, ConnectionError
+        ):
+            messages.append("Server busy. Please wait a moment and try again.")
+            if self._check_runtime_exception:
+                messages.append(
+                    f"{self._check_runtime_exception.__class__.__name__}: {self._check_runtime_exception}"
+                )
+            if self._fix_runtime_exception:
+                messages.append(
+                    f"{self._fix_runtime_exception.__class__.__name__}: {self._fix_runtime_exception}"
+                )
+        elif isinstance(self._check_runtime_exception, TimeoutError) or isinstance(
+            self._fix_runtime_exception, TimeoutError
+        ):
             messages.append(
-                "Validation Error: {}".format(self._check_runtime_exception)
+                """
+                Timeout occured while waiting for results. The operation
+                will finish, but you will need to re-validate to see the
+                results.
+            """
             )
+            messages.append(
+                """
+                For expensive operations, you may want to break up the
+                operation into smaller batches by selecting items from
+                the details panel and executing the operation on the
+                selected items.
+            """
+            )
+            if self._check_runtime_exception:
+                messages.append(
+                    f"{self._check_runtime_exception.__class__.__name__}: {self._check_runtime_exception}"
+                )
+            if self._fix_runtime_exception:
+                messages.append(
+                    f"{self._fix_runtime_exception.__class__.__name__}: {self._fix_runtime_exception}"
+                )
+        else:
+            if self._check_runtime_exception:
+                messages.append(f"Validation Error: {self._check_runtime_exception}")
+
+            if self._fix_runtime_exception:
+                messages.append(f"Fix Error: {self._fix_runtime_exception}")
 
-        if self._fix_runtime_exception:
-            messages.append("Fix Error: {}".format(self._fix_runtime_exception))
+            if not self._check_runtime_exception and not self._fix_executed:
+                # Only include the validation error message if both check and fix functions
+                # executed successfully.
+                if self.error_message:
+                    messages.append(self.error_message)
 
-        if not self._check_runtime_exception and not self._fix_executed:
-            # Only include the validation error message if both check and fix functions
-            # executed successfully.
-            if self.error_message:
-                messages.append(self.error_message)
+            if not self.errors and self.error_count > 0:
+                messages.append(f"Found ({self.error_count:,}) errors.")
 
         return messages
 
@@ -547,6 +599,7 @@ def exec_check(self, force=False):
             # Set the rule state to indicate that the check function not run
             self._valid = None
             self._error_items = None
+            self._error_count = 0
             result = None
         else:
             # Run the check function
@@ -559,16 +612,22 @@ def exec_check(self, force=False):
                     self._check_runtime_exception = runtime_error
                     self._valid = False
                     self._error_items = None
+                    self._error_count = 0
                     result = None
+                    # Raise exception if it is fatal
+                    if isinstance(runtime_error, (ConnectionError, TimeoutError)):
+                        raise runtime_error
             elif self.manual:
                 # This is a manual check. It is considered valid if the user has checked it off.
                 self._valid = self.manual_checked
                 self._error_items = None
+                self._error_count = 0
                 result = None
             else:
                 # This rule does not have a check function but it does have a fix
                 self._valid = True
                 self._error_items = None
+                self._error_count = 0
                 result = None
 
         return result
@@ -637,7 +696,10 @@ def exec_fix(self, pre_validate=True, force=False):
 
         # Get the key-word arguments and set the errors to pass to the fix function
         kwargs = self.get_kwargs()
-        kwargs["errors"] = self.errors
+        if pre_validate:
+            # Only pass the errors list if pre_validate is True. Otherwise, the
+            # error list could be stale
+            kwargs["errors"] = self.errors
 
         try:
             fix_result = self.fix_func(**kwargs)
@@ -646,6 +708,9 @@ def exec_fix(self, pre_validate=True, force=False):
         except Exception as runtime_error:
             self._fix_runtime_exception = runtime_error
             fix_result = False
+            # Raise exception if it is fatal
+            if isinstance(runtime_error, (ConnectionError, TimeoutError)):
+                raise runtime_error
 
         return fix_result
 
@@ -654,6 +719,7 @@ def reset(self):
 
         self._valid = None
         self._error_items = None
+        self._error_count = 0
         self._fix_executed = False
         self._check_runtime_exception = None
         self._fix_runtime_exception = None
@@ -695,6 +761,10 @@ def _process_check_result(self, result):
 
             self._valid = sanitized_result["is_valid"]
             self._error_items = sanitized_result["errors"]
+            if self._error_items:
+                self._error_count = len(self._error_items)
+            else:
+                self._error_count = sanitized_result.get("error_count", 0)
 
         elif isinstance(sanitized_result, object):
             for field in required_fields:
@@ -707,6 +777,10 @@ def _process_check_result(self, result):
 
             self._valid = sanitized_result.is_valid
             self._error_items = sanitized_result.errors
+            if self._error_items:
+                self._error_count = len(self._error_items)
+            elif hasattr(sanitized_result, "error_count"):
+                self._error_count = sanitized_result.error_count
 
         else:
             raise TypeError(

python/tk_multi_data_validation/api/manager.py

@@ -227,18 +227,27 @@ def validate(self):
         :rtype: bool
         """
 
+        success = True
+
         if self.notifier:
             self.notifier.validate_all_begin.emit(list(self.rules))
 
         try:
             # Reset the manager state before performing validation
             self.reset()
             self.validate_rules(self.rules, emit_signals=False)
+            success = not self.__errors
+        except Exception as validate_error:
+            if self.notifier:
+                self.notifier.validation_error.emit(validate_error)
+                success = False
+            else:
+                raise validate_error
         finally:
             if self.notifier:
                 self.notifier.validate_all_finished.emit()
 
-        return not self.__errors
+        return success
 
     @sgtk.LogManager.log_timing
     def validate_rules(self, rules, fetch_dependencies=True, emit_signals=True):
@@ -271,6 +280,11 @@ def validate_rules(self, rules, fetch_dependencies=True, emit_signals=True):
             self._process_rules(
                 rules, fetch_dependencies, emit_signals, self.validate_rule
             )
+        except Exception as validate_error:
+            if emit_signals and self.notifier:
+                self.notifier.validation_error.emit(validate_error)
+            else:
+                raise validate_error
         finally:
             if emit_signals and self.notifier:
                 self.notifier.validate_all_finished.emit()
@@ -316,6 +330,7 @@ def validate_rule(self, rule, emit_signals=True):
 
         return rule.valid
 
+    @sgtk.LogManager.log_timing
     def resolve(self, pre_validate=True, post_validate=False, retry_until_success=True):
         """
         Resolve the current data violations found by the validate method.
@@ -405,7 +420,12 @@ def resolve(self, pre_validate=True, post_validate=False, retry_until_success=Tr
                 self._logger.debug(
                     "Failed to resolve after max retry attempts. There may be a rule dependecy cycle."
                 )
-
+        except Exception as resolve_error:
+            if self.notifier:
+                self.notifier.validation_error.emit(resolve_error)
+                success = False
+            else:
+                raise resolve_error
         finally:
             if self.notifier:
                 self.notifier.resolve_all_finished.emit()
@@ -442,6 +462,11 @@ def resolve_rules(
                 emit_signals,
                 lambda rule: self.resolve_rule(rule, pre_validate=pre_validate),
             )
+        except Exception as resolve_error:
+            if emit_signals and self.notifier:
+                self.notifier.validation_error.emit(resolve_error)
+            else:
+                raise resolve_error
         finally:
             if emit_signals and self.notifier:
                 self.notifier.resolve_all_finished.emit()
@@ -469,14 +494,6 @@ def resolve_rule(self, rule, pre_validate=True, emit_signals=True):
                 success = True
             else:
                 success = False
-
-        except Exception as resolve_error:
-            self._logger.error(
-                "Failed to resolve rule {id}\n{error}".format(id=rule.id),
-                error=resolve_error,
-            )
-            success = False
-
         finally:
             if emit_signals and self.notifier:
                 self.notifier.resolve_rule_finished.emit(rule)

python/tk_multi_data_validation/dialog.py

@@ -178,6 +178,11 @@ def _connect_signals(self):
         self._manager.notifier.resolve_rule_finished.connect(
             self._validation_widget.fix_rule_finished
         )
+        self._manager.notifier.validation_error.connect(
+            lambda exception: self._validation_widget.show_validation_error(
+                text=f"{exception.__class__.__name__}: {exception}"
+            )
+        )
 
         # ------------------------------------------------------------
         # ValidationWidget signals connected to this dialog

python/tk_multi_data_validation/utils/validation_notifier.py

@@ -27,3 +27,4 @@ class ValidationNotifier(QtCore.QObject):
     resolve_all_finished = QtCore.Signal()
     about_to_open_msg_box = QtCore.Signal()
     msg_box_closed = QtCore.Signal()
+    validation_error = QtCore.Signal(Exception)

python/tk_multi_data_validation/widgets/validation_widget.py

@@ -478,6 +478,31 @@ def get_active_rules(self):
 
         return rules
 
+    def show_validation_error(self, show=True, text=None):
+        """
+        Show the validation warning.
+
+        The validation warning indicates that the scene has changed since the last validation.
+        Showing the validation warning will display a warning icon and text describing the
+        warning.
+
+        :param show: True will show the validation warning, False will hide it.
+        :type show: bool
+        :param text: Additional warning details to display.
+        :type text: str
+        """
+
+        if not show or text is None:
+            self._error_msg_label.setText("")
+            self._error_msg_widget.hide()
+        else:
+            current_text = self._error_msg_label.text()
+            error_text = f"<span style='color:#EB5555;'><b>{text}</b></span>"
+            if current_text:
+                error_text = f"{current_text}<br>{error_text}"
+            self._error_msg_label.setText(error_text)
+            self._error_msg_widget.show()
+
     def show_validation_warning(self, show=True, text=None):
         """
         Show the validation warning.
@@ -701,6 +726,23 @@ def _setup_ui(self):
             ],
         )
 
+        self._error_msg_label = SGQLabel(self)
+        self._error_msg_label.setWordWrap(True)
+        self._error_msg_label.setSizePolicy(
+            QtGui.QSizePolicy(QtGui.QSizePolicy.Preferred, QtGui.QSizePolicy.Maximum)
+        )
+        self._error_msg_label.setAlignment(QtCore.Qt.AlignLeft | QtCore.Qt.AlignVCenter)
+        self._error_msg_widget = SGQWidget(
+            self,
+            child_widgets=[
+                self._error_msg_label,
+            ],
+        )
+        self._error_msg_label.setSizePolicy(
+            QtGui.QSizePolicy(QtGui.QSizePolicy.Expanding, QtGui.QSizePolicy.Maximum)
+        )
+        self._error_msg_widget.hide()
+
         # -----------------------------------------------------
         # Bottom toolbar
         #
@@ -740,6 +782,7 @@ def _setup_ui(self):
         # Layouts and main widget
         #
 
+        self._error_msg_widget.layout().setContentsMargins(10, 0, 10, 10)
         self._toolbar_widget.layout().setContentsMargins(10, 0, 10, 0)
         self._content_widget.layout().setContentsMargins(0, 0, 0, 0)
         self._footer_widget.layout().setContentsMargins(10, 0, 10, 0)
@@ -751,6 +794,7 @@ def _setup_ui(self):
 
         self.add_widgets(
             [
+                self._error_msg_widget,
                 self._toolbar_widget,
                 self._content_widget,
                 self._footer_widget,
@@ -1468,6 +1512,7 @@ def validate_all_begin(self, rules=None):
             self.stop_event_listening.emit()
 
         self._is_validating_all = True
+        self.show_validation_error(show=False)
         self._start_progress(rules, "Begin validating...")
 
     def validate_all_finished(self):
@@ -1527,6 +1572,7 @@ def fix_all_begin(self, rules=None):
             self.stop_event_listening.emit()
 
         self._is_fixing_all = True
+        self.show_validation_error(show=False)
         self._start_progress(rules, "Begin fixing...")
 
     def fix_all_finished(self):