Merge pull request #271 from MerginMaps/python-37-tests

Move tests to version 3.8 + fix tempdir handling in older python

Author: MarcelGeo

.github/workflows/autotests.yml

@@ -19,7 +19,7 @@ jobs:
 
       - uses: actions/setup-python@v2
         with:
-          python-version: '3.x'
+          python-version: "3.8"
 
       - name: Install python package dependencies
         run: |

mergin/client_pull.py

@@ -22,7 +22,7 @@
 
 from .common import CHUNK_SIZE, ClientError
 from .merginproject import MerginProject
-from .utils import save_to_file
+from .utils import cleanup_tmp_dir, save_to_file
 
 
 # status = download_project_async(...)
@@ -145,7 +145,7 @@ def download_project_async(mc, project_path, directory, project_version=None):
     mp.log.info("--- version: " + mc.user_agent_info())
     mp.log.info(f"--- start download {project_path}")
 
-    tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-", ignore_cleanup_errors=True, delete=True)
+    tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-")
 
     try:
         # check whether we download the latest version or not
@@ -250,7 +250,7 @@ def download_project_finalize(job):
     # final update of project metadata
     job.mp.update_metadata(job.project_info)
 
-    job.tmp_dir.cleanup()
+    cleanup_tmp_dir(job.mp, job.tmp_dir)
 
 
 def download_project_cancel(job):
@@ -263,6 +263,7 @@ def download_project_cancel(job):
     job.is_cancelled = True
     job.executor.shutdown(wait=True)
     job.mp.log.info("--- download cancelled")
+    cleanup_tmp_dir(job.mp, job.tmp_dir)
 
 
 class UpdateTask:
@@ -424,7 +425,7 @@ def pull_project_async(mc, directory):
     # then we just download the whole file
     _pulling_file_with_diffs = lambda f: "diffs" in f and len(f["diffs"]) != 0
 
-    tmp_dir = tempfile.TemporaryDirectory(prefix="mm-pull-", ignore_cleanup_errors=True, delete=True)
+    tmp_dir = tempfile.TemporaryDirectory(prefix="mm-pull-")
     pull_changes = mp.get_pull_changes(server_info["files"])
     mp.log.debug("pull changes:\n" + pprint.pformat(pull_changes))
     fetch_files = []
@@ -550,6 +551,7 @@ def pull_project_cancel(job):
     job.is_cancelled = True
     job.executor.shutdown(wait=True)
     job.mp.log.info("--- pull cancelled")
+    cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
 
 
 class FileToMerge:
@@ -637,6 +639,7 @@ def pull_project_finalize(job: PullJob):
     except Exception as e:
         job.mp.log.error("Failed to apply pull changes: " + str(e))
         job.mp.log.info("--- pull aborted")
+        cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
         raise ClientError("Failed to apply pull changes: " + str(e))
 
     job.mp.update_metadata(job.project_info)
@@ -646,7 +649,7 @@ def pull_project_finalize(job: PullJob):
     else:
         job.mp.log.info("--- pull finished -- at version " + job.mp.version())
 
-    job.tmp_dir.cleanup()  # delete our temporary dir and all its content
+    cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
     return conflicts
 
 

mergin/client_push.py

@@ -26,6 +26,7 @@
 from .common import UPLOAD_CHUNK_SIZE, ClientError
 from .merginproject import MerginProject
 from .editor import filter_changes
+from .utils import cleanup_tmp_dir
 
 
 class UploadJob:
@@ -131,7 +132,7 @@ def push_project_async(mc, directory):
     changes = filter_changes(mc, project_info, changes)
     mp.log.debug("push changes:\n" + pprint.pformat(changes))
 
-    tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-", ignore_cleanup_errors=True, delete=True)
+    tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-")
 
     # If there are any versioned files (aka .gpkg) that are not updated through a diff,
     # we need to make a temporary copy somewhere to be sure that we are uploading full content.
@@ -267,6 +268,7 @@ def push_project_finalize(job):
             job.transferred_size, job.total_size
         )
         job.mp.log.error("--- push finish failed! " + error_msg)
+        cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
         raise ClientError("Upload error: " + error_msg)
 
     if with_upload_of_files:
@@ -310,6 +312,7 @@ def push_project_finalize(job):
                 job.mp.log.info("cancel response: " + resp_cancel.msg)
             except ClientError as err2:
                 job.mp.log.info("cancel response: " + str(err2))
+            cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
             raise err
 
     job.mp.update_metadata(job.server_resp)
@@ -318,10 +321,10 @@ def push_project_finalize(job):
     except Exception as e:
         job.mp.log.error("Failed to apply push changes: " + str(e))
         job.mp.log.info("--- push aborted")
+        cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
         raise ClientError("Failed to apply push changes: " + str(e))
 
-    job.tmp_dir.cleanup()  # delete our temporary dir and all its content
-
+    cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
     remove_diff_files(job)
 
     job.mp.log.info("--- push finished - new project version " + job.server_resp["version"])
@@ -362,7 +365,9 @@ def push_project_cancel(job):
         job.server_resp = resp_cancel.msg
     except ClientError as err:
         job.mp.log.error("--- push cancelling failed! " + str(err))
+        cleanup_tmp_dir(job.mp, job.tmp_dir)
         raise err
+    cleanup_tmp_dir(job.mp, job.tmp_dir)  # delete our temporary dir and all its content
     job.mp.log.info("--- push cancel response: " + str(job.server_resp))
 
 

mergin/test/test_client.py

@@ -2266,8 +2266,11 @@ def test_clean_diff_files(mc):
     shutil.copy(mp.fpath("inserted_1_A.gpkg"), mp.fpath(f_updated))
     mc.push_project(project_dir)
 
-    diff_files = glob.glob("*-diff-*", root_dir=os.path.split(mp.fpath_meta("inserted_1_A.gpkg"))[0])
+    # Get the directory path
+    directory = os.path.split(mp.fpath_meta("inserted_1_A.gpkg"))[0]
+    diff_files = [f for f in os.listdir(directory) if "-diff-" in f]
 
+    # Assert that no matching files are found
     assert diff_files == []
 
 

mergin/utils.py

@@ -6,6 +6,7 @@
 import sqlite3
 from datetime import datetime
 from pathlib import Path
+import tempfile
 from .common import ClientError
 
 
@@ -201,7 +202,7 @@ def conflicted_copy_file_name(path, user, version):
     head, tail = os.path.split(os.path.normpath(path))
     ext = "".join(Path(tail).suffixes)
     file_name = tail.replace(ext, "")
-    # in case of QGIS project files we have to add "~" (tilde) to suffix
+    # in case of QGIS project files we TemporaryDirectoryhave to add "~" (tilde) to suffix
     # to avoid having several QGIS project files inside Mergin Maps project.
     # See https://github.com/lutraconsulting/qgis-mergin-plugin/issues/382
     # for more details
@@ -294,3 +295,17 @@ def bytes_to_human_size(bytes: int):
         return f"{round( bytes / 1024.0 / 1024.0 / 1024.0, precision )} GB"
     else:
         return f"{round( bytes / 1024.0 / 1024.0 / 1024.0 / 1024.0, precision )} TB"
+
+
+def cleanup_tmp_dir(mp, tmp_dir: tempfile.TemporaryDirectory):
+    """
+    Remove temporary from tempfile.TemporaryDirectory instance
+    This is needed beacause ignore_clanup_errors is not accepted under < Python 3.10
+    """
+
+    try:
+        tmp_dir.cleanup()
+    except PermissionError:
+        mp.log.warning(f"Permission error during tmp dir cleanup: {tmp_dir.name}")
+    except Exception as e:
+        mp.log.error(f"Error during tmp dir cleanup: {tmp_dir.name}: {e}")