Skip to content

Store pull files in temp directory #253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 6, 2025
Merged

Store pull files in temp directory #253

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56d8b8b
change directory where pull files are stored
JanCaha Jul 23, 2025
52bd00c
move temp_dir to TemporaryDirectory instance
JanCaha Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mergin/client_pull.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,8 @@ 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

temp_dir = mp.fpath_meta(f"fetch_{local_version}-{server_version}")
temp_dir_mergin = os.path.join(tempfile.gettempdir(), f"mergin-{mp.project_id()}")
temp_dir = os.path.join(temp_dir_mergin, f"fetch_{local_version}-{server_version}")
Copy link
Contributor

@wonder-sk wonder-sk Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using e.g. tempfile.TemporaryDirectory(prefix="mm-pull-") here and keeping the tmp dir reference in the pull job? That should be safer and always unique. We could also use it with delete=True flag and even ignore_cleanup_errors=True - and we don't need to worry about its cleanup, even in case of exceptions etc.

Copy link
Contributor

@MarcelGeo MarcelGeo Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good alternative to mkdtemp. I think we can not use with context here.

I think we can manually cleanup temp dir on the finish of pull (rmtree(temp_dir, ignore_errors=True) string alternatively use cleanup() method of TemporaryDirectory). But sure, just 🍒

there is qustionable also "unfinish_pull_dir" (where also some pull related staff is stored) and ".cache" dir (which is ok I think).

Copy link
Contributor

@wonder-sk wonder-sk Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfinished pull dir should not be a problem - if there is a failure during pull, we'll create the unfinished pull directory, and ask user to restart qgis and try again (so that some open sqlite connections get closed) - it is not something that's likely to interfere with onedrive etc...

os.makedirs(temp_dir, exist_ok=True)
pull_changes = mp.get_pull_changes(server_info["files"])
mp.log.debug("pull changes:\n" + pprint.pformat(pull_changes))
Expand Down