Fixing many Python issues reported in our automatic exception tracking (

OpenShot#2482)

* Ignoring lock file removal failures

* Adding retry logic on creating and destroying lock file (and ignoring errors related to that). These are very noisy errors, and this should silience them.

* Protecting Track lookup when inserting new track above/below (sometimes a track ID cannot be found in the project data which causes a failure here)

* Adding additional logging when history keys are skipped during loading/saving (to reduce file size of OSP project file)

* Disable Title editor save button until a template is selected (which prevents an error)

* Bumping version to 2.4.3-dev2

src/classes/info.py

@@ -29,7 +29,7 @@
 
 from PyQt5.QtCore import QDir
 
-VERSION = "2.4.3-dev1"
+VERSION = "2.4.3-dev2"
 MINIMUM_LIBOPENSHOT_VERSION = "0.2.2"
 DATE = "20180922000000"
 NAME = "openshot-qt"

src/classes/updates.py

@@ -153,11 +153,15 @@ def load_history(self, project):
             action.load_json(json.dumps(actionDict))
             if action.type != "load" and action.key[0] != "history":
                 self.redoHistory.append(action)
+            else:
+                log.info("Loading redo history, skipped key: %s" % str(action.key))
         for actionDict in history.get("undo", []):
             action = UpdateAction()
             action.load_json(json.dumps(actionDict))
             if action.type != "load" and action.key[0] != "history":
                 self.actionHistory.append(action)
+            else:
+                log.info("Loading undo history, skipped key: %s" % str(action.key))
 
         # Notify watchers of new status
         self.update_watchers()
@@ -174,10 +178,14 @@ def save_history(self, project, history_length):
             if action.type != "load" and action.key[0] != "history":
                 actionDict = json.loads(action.json())
                 redo_list.append(actionDict)
+            else:
+                log.info("Saving redo history, skipped key: %s" % str(action.key))
         for action in self.actionHistory[-history_length_int:]:
             if action.type != "load" and action.key[0] != "history":
                 actionDict = json.loads(action.json())
                 undo_list.append(actionDict)
+            else:
+                log.info("Saving undo, skipped key: %s" % str(action.key))
 
         # Set history data in project
         self.ignore_history = True

src/windows/main_window.py

@@ -34,6 +34,7 @@
 import webbrowser
 from uuid import uuid4
 from copy import deepcopy
+from time import sleep
 
 from PyQt5.QtCore import *
 from PyQt5.QtGui import QIcon, QCursor, QKeySequence
@@ -245,18 +246,31 @@ def create_lock_file(self):
             # Normal startup, clear thumbnails
             self.clear_all_thumbnails()
 
-        # Create lock file
-        with open(lock_path, 'w') as f:
-            f.write(lock_value)
+        # Write lock file (try a few times if failure)
+        attempts = 5
+        while attempts > 0:
+            try:
+                # Create lock file
+                with open(lock_path, 'w') as f:
+                    f.write(lock_value)
+                break
+            except Exception:
+                attempts -= 1
+                sleep(0.25)
 
     def destroy_lock_file(self):
         """Destroy the lock file"""
         lock_path = os.path.join(info.USER_PATH, ".lock")
 
-        # Check if it already exists
-        if os.path.exists(lock_path):
-            # Remove file
-            os.remove(lock_path)
+        # Remove file (try a few times if failure)
+        attempts = 5
+        while attempts > 0:
+            try:
+                os.remove(lock_path)
+                break
+            except Exception:
+                attempts -= 1
+                sleep(0.25)
 
     def tail_file(self, f, n, offset=None):
         """Read the end of a file (n number of lines)"""
@@ -989,33 +1003,35 @@ def actionAddTrackAbove_trigger(self, event):
 
         # Get selected track data
         existing_track = Track.get(id=selected_layer_id)
+        if not existing_track:
+            # Log error and fail silently
+            log.error('No track object found with id: %s' % selected_layer_id)
+            return
         selected_layer_number = int(existing_track.data["number"])
 
-        # log.info("Adding track above #{} (id {})".format(selected_layer_number, selected_layer_id))
-
         # Loop through tracks above insert point (in descending order), renumbering layers
         for existing_layer in list(reversed(range(selected_layer_number+1, max_track_number))):
             existing_track = Track.get(number=existing_layer)
-            # log.info("Renumbering track id {} from {} to {}".format(existing_track.data["id"], existing_layer, existing_layer+1))
+            if not existing_track:
+                # Log error and fail silently, and continue
+                log.error('No track object found with number: %s' % existing_layer)
+                continue
             existing_track.data["number"] = existing_layer + 1
             existing_track.save()
 
             # Loop through clips and transitions for track, moving up to new layer
             for clip in Clip.filter(layer=existing_layer):
-                # log.info("Moving clip id {} from layer {} to {}".format(clip.data["id"], int(clip.data["layer"]), int(clip.data["layer"])+1))
                 clip.data["layer"] = int(clip.data["layer"]) + 1
                 clip.save()
 
             for trans in Transition.filter(layer=existing_layer):
-                # log.info("Moving transition id {} from layer {} to {}".format(trans.data["id"], int(trans.data["layer"]), int(trans.data["layer"])+1))
                 trans.data["layer"] = int(trans.data["layer"]) + 1
                 trans.save()
 
         # Create new track at vacated layer
         track = Track()
         track.data = {"number": selected_layer_number+1, "y": 0, "label": "", "lock": False}
         track.save()
-        # log.info("Created new track id {} at layer number {}".format(track.data["id"], track.data["number"]))
 
     def actionAddTrackBelow_trigger(self, event):
         log.info("actionAddTrackBelow_trigger")
@@ -1026,33 +1042,35 @@ def actionAddTrackBelow_trigger(self, event):
 
         # Get selected track data
         existing_track = Track.get(id=selected_layer_id)
+        if not existing_track:
+            # Log error and fail silently
+            log.error('No track object found with id: %s' % selected_layer_id)
+            return
         selected_layer_number = int(existing_track.data["number"])
 
-        # log.info("Adding track below #{} (id {})".format(selected_layer_number, selected_layer_id))
-
         # Loop through tracks from insert point up (in descending order), renumbering layers
         for existing_layer in list(reversed(range(selected_layer_number, max_track_number))):
             existing_track = Track.get(number=existing_layer)
-            # log.info("Renumbering track id {} from {} to {}".format(existing_track.data["id"], existing_layer, existing_layer+1))
+            if not existing_track:
+                # Log error and fail silently, and continue
+                log.error('No track object found with number: %s' % existing_layer)
+                continue
             existing_track.data["number"] = existing_layer + 1
             existing_track.save()
 
             # Loop through clips and transitions for track, moving up to new layer
             for clip in Clip.filter(layer=existing_layer):
-                # log.info("Moving clip id {} from layer {} to {}".format(clip.data["id"], int(clip.data["layer"]), int(clip.data["layer"])+1))
                 clip.data["layer"] = int(clip.data["layer"]) + 1
                 clip.save()
 
             for trans in Transition.filter(layer=existing_layer):
-                # log.info("Moving transition id {} from layer {} to {}".format(trans.data["id"], int(trans.data["layer"]), int(trans.data["layer"])+1))
                 trans.data["layer"] = int(trans.data["layer"]) + 1
                 trans.save()
 
         # Create new track at vacated layer
         track = Track()
         track.data = {"number": selected_layer_number, "y": 0, "label": "", "lock": False}
         track.save()
-        # log.info("Created new track id {} at layer number {}".format(track.data["id"], track.data["number"]))
 
     def actionArrowTool_trigger(self, event):
         log.info("actionArrowTool_trigger")

src/windows/title_editor.py

@@ -108,6 +108,9 @@ def __init__(self, edit_file_path=None, duplicate=False):
         self.titlesTreeView = TitlesListView(self)
         self.verticalLayout.addWidget(self.titlesTreeView)
 
+        # Disable Save button on window load
+        self.buttonBox.button(self.buttonBox.Save).setEnabled(False)
+
         # If editing existing title svg file
         if self.edit_file_path:
             # Hide list of templates
@@ -300,6 +303,9 @@ def load_svg_template(self):
             self.btnFont.setEnabled(False)
             self.btnFontColor.setEnabled(False)
 
+        # Enable Save button when a template is selected
+        self.buttonBox.button(self.buttonBox.Save).setEnabled(True)
+
     def writeToFile(self, xmldoc):
         '''writes a new svg file containing the user edited data'''