diff --git a/.github/workflows/autotests.yml b/.github/workflows/autotests.yml index fc658a58..c292728a 100644 --- a/.github/workflows/autotests.yml +++ b/.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: | diff --git a/mergin/client_pull.py b/mergin/client_pull.py index c3eb422f..264f4708 100644 --- a/mergin/client_pull.py +++ b/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 diff --git a/mergin/client_push.py b/mergin/client_push.py index 90fbd19a..54afdd40 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -19,6 +19,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: @@ -124,7 +125,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. @@ -260,6 +261,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: @@ -275,7 +277,7 @@ def push_project_finalize(job): f"Upload details: {len(job.upload_queue_items)} file chunks, total size {job.total_size} bytes." ) # server returns various error messages with filename or something generic - # it would be better if it returned list of failed files (and reasons) whenever possible + # it would be better if it returned list of failed files (and reasons) whenever possible job.mp.log.error("--- push finish failed! " + str(err)) # if push finish fails, the transaction is not killed, so we @@ -286,6 +288,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) @@ -294,10 +297,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"]) @@ -319,7 +322,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)) diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py index be13ffb1..58278b52 100644 --- a/mergin/test/test_client.py +++ b/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 == [] diff --git a/mergin/utils.py b/mergin/utils.py index 7b6c9f9f..c9b480a0 100644 --- a/mergin/utils.py +++ b/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}")