From 1d26f6a15056a3b346202d5a328c4191eff8b5cc Mon Sep 17 00:00:00 2001 From: DanChov Date: Sun, 7 Sep 2025 14:35:15 +0200 Subject: [PATCH 1/7] feat(push): extend 502/504 failure logging with upload summary and file list; include gpkg diff size and change count via safe helper; add real-diff test; no change to success path --- mergin/client_push.py | 55 ++++++++++++++-- mergin/test/test_push_logging.py | 108 +++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 mergin/test/test_push_logging.py diff --git a/mergin/client_push.py b/mergin/client_push.py index d52ff5a9..d90e9bfb 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -268,14 +268,31 @@ def push_project_finalize(job): resp = job.mc.post("/v1/project/push/finish/%s" % job.transaction_id) job.server_resp = json.load(resp) except ClientError as err: - # Log additional metadata on server error 502 or 504 - if hasattr(err, "http_error") and err.http_error in (502, 504): + # Log additional metadata on server error 502 or 504 (extended logging only) + http_code = getattr(err, "http_error", None) + if http_code in (502, 504): job.mp.log.error( - f"Push failed with HTTP error {err.http_error}. " + f"Push failed with HTTP error {http_code}. " f"Upload details: {len(job.upload_queue_items)} file chunks, total size {job.total_size} bytes." ) + job.mp.log.error("Files:") + for f in job.changes.get("added", []) + job.changes.get("updated", []): + path = f.get("path", "") + size = f.get("size", "?") + if "diff" in f: + diff_info = f.get("diff", {}) + diff_size = diff_info.get("size", "?") + # best-effort: number of geodiff changes (if available) + changes_cnt = _geodiff_changes_count(job.mp, diff_info.get("path")) + if changes_cnt is None: + job.mp.log.error(f" - {path}, size={size}, diff_size={diff_size}, changes=n/a") + else: + job.mp.log.error(f" - {path}, size={size}, diff_size={diff_size}, changes={changes_cnt}") + else: + job.mp.log.error(f" - {path}, size={size}") + # 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 @@ -303,6 +320,36 @@ def push_project_finalize(job): job.mp.log.info("--- push finished - new project version " + job.server_resp["version"]) +def _geodiff_changes_count(mp, diff_rel_path): + """ + Best-effort: return number of changes in the .gpkg diff (int) or None. + Never raises – diagnostics/logging must not fail. + """ + if not diff_rel_path: + return None + try: + diff_abs = mp.fpath_meta(diff_rel_path) + except Exception: + return None + + try: + from pygeodiff import GeoDiff + return GeoDiff().changes_count(diff_abs) + except Exception: + pass + + try: + import geodiff # optional fallback + if hasattr(geodiff, "changes_count"): + try: + return geodiff.changes_count(diff_abs) + except Exception: + return None + except Exception: + return None + + return None + def push_project_cancel(job): """ To be called (from main thread) to cancel a job that has uploads in progress. diff --git a/mergin/test/test_push_logging.py b/mergin/test/test_push_logging.py new file mode 100644 index 00000000..19dea5fb --- /dev/null +++ b/mergin/test/test_push_logging.py @@ -0,0 +1,108 @@ +import logging +import tempfile +from pathlib import Path +from types import SimpleNamespace + +import pytest + + +@pytest.mark.parametrize("status_code", [502, 504]) +def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code): + # geodiff is required – if missing, skip the test gracefully + pytest.importorskip("pygeodiff") + from pygeodiff import GeoDiff + + from mergin.client_push import push_project_finalize + from mergin.common import ClientError + + # --- exact paths according to your repo structure --- + data_dir = Path(__file__).resolve().parent / "test_data" + base = data_dir / "base.gpkg" + modified = data_dir / "inserted_1_A.gpkg" + + # if something is missing, print the directory contents and FAIL + if not base.exists() or not modified.exists(): + print("[DEBUG] data_dir:", data_dir) + print("[DEBUG] data_dir.exists():", data_dir.exists()) + if data_dir.exists(): + print("[DEBUG] contents:", [p.name for p in sorted(data_dir.iterdir())]) + pytest.fail(f"Expected files are not available: base={base.exists()} inserted_1_A={modified.exists()}") + + # --- create a real .diff (changeset) --- + with tempfile.TemporaryDirectory(prefix="geodiff-") as tmpdir: + tmpdir = Path(tmpdir) + diff_path = tmpdir / "base_to_inserted_1_A.diff" + + gd = GeoDiff() + gd.create_changeset(str(base), str(modified), str(diff_path)) + diff_size = diff_path.stat().st_size + file_size = modified.stat().st_size + + # --- logger captured by caplog --- + logger = logging.getLogger("mergin_test") + logger.setLevel(logging.DEBUG) + logger.propagate = True + + # --- fake mc.post: /finish -> 5xx, /cancel -> ok --- + def fake_post(url, *args, **kwargs): + if "/finish/" in url: + err = ClientError("Gateway error") + setattr(err, "http_error", status_code) + raise err + if "/cancel/" in url: + return SimpleNamespace(msg="cancelled") + return SimpleNamespace(msg="ok") + + # --- fake future/executor (no exceptions) --- + fake_future = SimpleNamespace(done=lambda: True, exception=lambda: None, running=lambda: False) + fake_executor = SimpleNamespace(shutdown=lambda wait=True: None) + + # mp.fpath_meta should resolve the relative diff path to an absolute path + def fpath_meta(rel): + return str(diff_path) if rel == diff_path.name else rel + + # --- minimal job – only what finalize() uses --- + job = SimpleNamespace( + executor=fake_executor, + futures=[fake_future], + transferred_size=1234, + total_size=1234, + upload_queue_items=[1], + transaction_id="tx-1", + mp=SimpleNamespace( + log=logger, + update_metadata=lambda *a, **k: None, + apply_push_changes=lambda *a, **k: None, + fpath_meta=fpath_meta, + ), + mc=SimpleNamespace(post=fake_post), + changes={ + "added": [], + "updated": [{ + "path": modified.name, + "size": file_size, + "diff": {"path": diff_path.name, "size": diff_size}, + "chunks": [1], + }], + "removed": [], + }, + tmp_dir=SimpleNamespace(cleanup=lambda: None), + ) + + # capture ERROR-level logs from "mergin_test" + caplog.set_level(logging.ERROR, logger="mergin_test") + + # --- call production logic (expecting 5xx) --- + with pytest.raises(ClientError): + push_project_finalize(job) + + text = caplog.text + + assert f"Push failed with HTTP error {status_code}" in text + assert "Upload details:" in text + assert "Files:" in text + assert modified.name in text + assert f"size={file_size}" in text + assert f"diff_size={diff_size}" in text + # helper may compute changes, or log changes=n/a – accept both + assert ("changes=" in text) or ("changes=n/a" in text) From 83172696d3bfbf7cd0cce021d3d3d486833bcbed Mon Sep 17 00:00:00 2001 From: DanChov Date: Mon, 8 Sep 2025 16:07:46 +0200 Subject: [PATCH 2/7] black -l 120 --- mergin/client_push.py | 3 +++ mergin/test/test_push_logging.py | 14 +++++----- scripts/update_version.py | 6 ++--- setup.py | 44 +++++++++++++++----------------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index d90e9bfb..f8188fa4 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -334,12 +334,14 @@ def _geodiff_changes_count(mp, diff_rel_path): try: from pygeodiff import GeoDiff + return GeoDiff().changes_count(diff_abs) except Exception: pass try: import geodiff # optional fallback + if hasattr(geodiff, "changes_count"): try: return geodiff.changes_count(diff_abs) @@ -350,6 +352,7 @@ def _geodiff_changes_count(mp, diff_rel_path): return None + def push_project_cancel(job): """ To be called (from main thread) to cancel a job that has uploads in progress. diff --git a/mergin/test/test_push_logging.py b/mergin/test/test_push_logging.py index 19dea5fb..d6626a89 100644 --- a/mergin/test/test_push_logging.py +++ b/mergin/test/test_push_logging.py @@ -78,12 +78,14 @@ def fpath_meta(rel): mc=SimpleNamespace(post=fake_post), changes={ "added": [], - "updated": [{ - "path": modified.name, - "size": file_size, - "diff": {"path": diff_path.name, "size": diff_size}, - "chunks": [1], - }], + "updated": [ + { + "path": modified.name, + "size": file_size, + "diff": {"path": diff_path.name, "size": diff_size}, + "chunks": [1], + } + ], "removed": [], }, tmp_dir=SimpleNamespace(cleanup=lambda: None), diff --git a/scripts/update_version.py b/scripts/update_version.py index 184d6a8a..20206593 100644 --- a/scripts/update_version.py +++ b/scripts/update_version.py @@ -4,7 +4,7 @@ def replace_in_file(filepath, regex, sub): - with open(filepath, 'r') as f: + with open(filepath, "r") as f: content = f.read() content_new = re.sub(regex, sub, content, flags=re.M) @@ -15,14 +15,14 @@ def replace_in_file(filepath, regex, sub): dir_path = os.path.dirname(os.path.realpath(__file__)) parser = argparse.ArgumentParser() -parser.add_argument('--version', help='version to replace') +parser.add_argument("--version", help="version to replace") args = parser.parse_args() ver = args.version print("using version " + ver) about_file = os.path.join(dir_path, os.pardir, "mergin", "version.py") print("patching " + about_file) -replace_in_file(about_file, "__version__\s=\s\".*", "__version__ = \"" + ver + "\"") +replace_in_file(about_file, '__version__\s=\s".*', '__version__ = "' + ver + '"') setup_file = os.path.join(dir_path, os.pardir, "setup.py") print("patching " + setup_file) diff --git a/setup.py b/setup.py index 371014e9..2b4df73e 100644 --- a/setup.py +++ b/setup.py @@ -4,35 +4,31 @@ from setuptools import setup, find_packages setup( - name='mergin-client', - version='0.10.5', - url='https://github.com/MerginMaps/python-api-client', - license='MIT', - author='Lutra Consulting Ltd.', - author_email='info@merginmaps.com', - description='Mergin Maps utils and client', - long_description='Mergin Maps utils and client', - + name="mergin-client", + version="0.10.5", + url="https://github.com/MerginMaps/python-api-client", + license="MIT", + author="Lutra Consulting Ltd.", + author_email="info@merginmaps.com", + description="Mergin Maps utils and client", + long_description="Mergin Maps utils and client", packages=find_packages(), - - platforms='any', + platforms="any", install_requires=[ - 'python-dateutil==2.8.2', - 'pygeodiff==2.0.4', - 'pytz==2022.1', - 'click==8.1.3', + "python-dateutil==2.8.2", + "pygeodiff==2.0.4", + "pytz==2022.1", + "click==8.1.3", ], - entry_points={ - 'console_scripts': ['mergin=mergin.cli:cli'], + "console_scripts": ["mergin=mergin.cli:cli"], }, - classifiers=[ - 'Development Status :: 5 - Production/Stable', - 'Intended Audience :: Developers', - 'License :: OSI Approved :: MIT License', - 'Operating System :: OS Independent', - 'Programming Language :: Python :: 3' + "Development Status :: 5 - Production/Stable", + "Intended Audience :: Developers", + "License :: OSI Approved :: MIT License", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", ], - package_data={'mergin': ['cert.pem']} + package_data={"mergin": ["cert.pem"]}, ) From ed7e2760a71ed96fefb49a880d50d99261dc7d37 Mon Sep 17 00:00:00 2001 From: DanChov Date: Tue, 9 Sep 2025 10:25:28 +0200 Subject: [PATCH 3/7] eddited after rewiew: client_push.py removed unnecessary code, specified exceptions, added type hints test_push_logging.py changeed to use real job --- mergin/client_push.py | 26 +---- mergin/test/test_push_logging.py | 173 ++++++++++++++----------------- 2 files changed, 80 insertions(+), 119 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index f8188fa4..a1d96b61 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -320,39 +320,23 @@ def push_project_finalize(job): job.mp.log.info("--- push finished - new project version " + job.server_resp["version"]) -def _geodiff_changes_count(mp, diff_rel_path): +def _geodiff_changes_count(mp: MerginProject, diff_rel_path: str): """ Best-effort: return number of changes in the .gpkg diff (int) or None. Never raises – diagnostics/logging must not fail. """ - if not diff_rel_path: - return None + try: diff_abs = mp.fpath_meta(diff_rel_path) - except Exception: + except FileNotFoundError: return None try: - from pygeodiff import GeoDiff - + from pygeodiff import GeoDiff, GeoDiffLibError return GeoDiff().changes_count(diff_abs) - except Exception: - pass - - try: - import geodiff # optional fallback - - if hasattr(geodiff, "changes_count"): - try: - return geodiff.changes_count(diff_abs) - except Exception: - return None - except Exception: + except GeoDiffLibError: return None - return None - - def push_project_cancel(job): """ To be called (from main thread) to cancel a job that has uploads in progress. diff --git a/mergin/test/test_push_logging.py b/mergin/test/test_push_logging.py index d6626a89..46a8bf65 100644 --- a/mergin/test/test_push_logging.py +++ b/mergin/test/test_push_logging.py @@ -1,110 +1,87 @@ +from types import SimpleNamespace +from pathlib import Path import logging import tempfile -from pathlib import Path -from types import SimpleNamespace - import pytest +from pygeodiff import GeoDiff +from mergin.client_push import push_project_finalize, UploadJob +from mergin.common import ClientError @pytest.mark.parametrize("status_code", [502, 504]) -def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code): - # geodiff is required – if missing, skip the test gracefully - pytest.importorskip("pygeodiff") - from pygeodiff import GeoDiff - - from mergin.client_push import push_project_finalize - from mergin.common import ClientError - - # --- exact paths according to your repo structure --- +def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code, tmp_path): + # --- data --- data_dir = Path(__file__).resolve().parent / "test_data" base = data_dir / "base.gpkg" modified = data_dir / "inserted_1_A.gpkg" - - # if something is missing, print the directory contents and FAIL - if not base.exists() or not modified.exists(): - print("[DEBUG] data_dir:", data_dir) - print("[DEBUG] data_dir.exists():", data_dir.exists()) - if data_dir.exists(): - print("[DEBUG] contents:", [p.name for p in sorted(data_dir.iterdir())]) - pytest.fail(f"Expected files are not available: base={base.exists()} inserted_1_A={modified.exists()}") - - # --- create a real .diff (changeset) --- - with tempfile.TemporaryDirectory(prefix="geodiff-") as tmpdir: - tmpdir = Path(tmpdir) - diff_path = tmpdir / "base_to_inserted_1_A.diff" - - gd = GeoDiff() - gd.create_changeset(str(base), str(modified), str(diff_path)) - diff_size = diff_path.stat().st_size - file_size = modified.stat().st_size - - # --- logger captured by caplog --- - logger = logging.getLogger("mergin_test") - logger.setLevel(logging.DEBUG) - logger.propagate = True - - # --- fake mc.post: /finish -> 5xx, /cancel -> ok --- - def fake_post(url, *args, **kwargs): - if "/finish/" in url: - err = ClientError("Gateway error") - setattr(err, "http_error", status_code) - raise err - if "/cancel/" in url: + assert base.exists() + assert modified.exists() + + diff_path = tmp_path / "base_to_inserted_1_A.diff" + GeoDiff().create_changeset(str(base), str(modified), str(diff_path)) + diff_size = diff_path.stat().st_size + file_size = modified.stat().st_size + + # --- logger --- + logger = logging.getLogger("mergin_test") + logger.setLevel(logging.DEBUG) + logger.propagate = True + caplog.set_level(logging.ERROR, logger="mergin_test") + + # --- fake MP/MC len tam, kde treba --- + class MP: + def __init__(self, log): self.log = log + def update_metadata(self, *a, **k): pass + def apply_push_changes(self, *a, **k): pass + def fpath_meta(self, rel): return str(diff_path) if rel == diff_path.name else rel + + tx = "tx-1" + + class MC: + def post(self, url, *args, **kwargs): + if url.endswith(f"/v1/project/push/finish/{tx}"): + err = ClientError("Gateway error"); setattr(err, "http_error", status_code); raise err + if url.endswith(f"/v1/project/push/cancel/{tx}"): return SimpleNamespace(msg="cancelled") return SimpleNamespace(msg="ok") - # --- fake future/executor (no exceptions) --- - fake_future = SimpleNamespace(done=lambda: True, exception=lambda: None, running=lambda: False) - fake_executor = SimpleNamespace(shutdown=lambda wait=True: None) - - # mp.fpath_meta should resolve the relative diff path to an absolute path - def fpath_meta(rel): - return str(diff_path) if rel == diff_path.name else rel - - # --- minimal job – only what finalize() uses --- - job = SimpleNamespace( - executor=fake_executor, - futures=[fake_future], - transferred_size=1234, - total_size=1234, - upload_queue_items=[1], - transaction_id="tx-1", - mp=SimpleNamespace( - log=logger, - update_metadata=lambda *a, **k: None, - apply_push_changes=lambda *a, **k: None, - fpath_meta=fpath_meta, - ), - mc=SimpleNamespace(post=fake_post), - changes={ - "added": [], - "updated": [ - { - "path": modified.name, - "size": file_size, - "diff": {"path": diff_path.name, "size": diff_size}, - "chunks": [1], - } - ], - "removed": [], - }, - tmp_dir=SimpleNamespace(cleanup=lambda: None), - ) - - # capture ERROR-level logs from "mergin_test" - caplog.set_level(logging.ERROR, logger="mergin_test") - - # --- call production logic (expecting 5xx) --- - with pytest.raises(ClientError): - push_project_finalize(job) - - text = caplog.text - - assert f"Push failed with HTTP error {status_code}" in text - assert "Upload details:" in text - assert "Files:" in text - assert modified.name in text - assert f"size={file_size}" in text - assert f"diff_size={diff_size}" in text - # helper may compute changes, or log changes=n/a – accept both - assert ("changes=" in text) or ("changes=n/a" in text) + tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-test-") + + # --- real UploadJob objekt --- + job = UploadJob( + project_path="u/p", + changes={ + "added": [], + "updated": [{ + "path": modified.name, + "size": file_size, + "diff": {"path": diff_path.name, "size": diff_size}, + "chunks": [1], + }], + "removed": [], + }, + transaction_id=tx, + mp=MP(logger), + mc=MC(), + tmp_dir=tmp_dir, + ) + + # nastav to, čo finalize() očakáva + job.total_size = 1234 + job.transferred_size = 1234 + job.upload_queue_items = [1] # len pre log „Upload details“ + job.executor = SimpleNamespace(shutdown=lambda wait=True: None) + job.futures = [SimpleNamespace(done=lambda: True, exception=lambda: None, running=lambda: False)] + job.server_resp = {"version": "n/a"} # aj tak sa 5xx vyhodí skôr + + with pytest.raises(ClientError): + push_project_finalize(job) + + text = caplog.text + assert f"Push failed with HTTP error {status_code}" in text + assert "Upload details:" in text + assert "Files:" in text + assert modified.name in text + assert f"size={file_size}" in text + assert f"diff_size={diff_size}" in text + assert ("changes=" in text) or ("changes=n/a" in text) \ No newline at end of file From 70d3edaad6f04a166d83046ed6f2e160f3e10cc5 Mon Sep 17 00:00:00 2001 From: DanChov Date: Tue, 9 Sep 2025 10:30:48 +0200 Subject: [PATCH 4/7] black -l 120 --- mergin/client_push.py | 2 ++ mergin/test/test_push_logging.py | 36 +++++++++++++++++++++----------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index a1d96b61..3a099292 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -333,10 +333,12 @@ def _geodiff_changes_count(mp: MerginProject, diff_rel_path: str): try: from pygeodiff import GeoDiff, GeoDiffLibError + return GeoDiff().changes_count(diff_abs) except GeoDiffLibError: return None + def push_project_cancel(job): """ To be called (from main thread) to cancel a job that has uploads in progress. diff --git a/mergin/test/test_push_logging.py b/mergin/test/test_push_logging.py index 46a8bf65..1e6394e9 100644 --- a/mergin/test/test_push_logging.py +++ b/mergin/test/test_push_logging.py @@ -8,6 +8,7 @@ from mergin.client_push import push_project_finalize, UploadJob from mergin.common import ClientError + @pytest.mark.parametrize("status_code", [502, 504]) def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code, tmp_path): # --- data --- @@ -30,17 +31,26 @@ def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code, tmp_path): # --- fake MP/MC len tam, kde treba --- class MP: - def __init__(self, log): self.log = log - def update_metadata(self, *a, **k): pass - def apply_push_changes(self, *a, **k): pass - def fpath_meta(self, rel): return str(diff_path) if rel == diff_path.name else rel + def __init__(self, log): + self.log = log + + def update_metadata(self, *a, **k): + pass + + def apply_push_changes(self, *a, **k): + pass + + def fpath_meta(self, rel): + return str(diff_path) if rel == diff_path.name else rel tx = "tx-1" class MC: def post(self, url, *args, **kwargs): if url.endswith(f"/v1/project/push/finish/{tx}"): - err = ClientError("Gateway error"); setattr(err, "http_error", status_code); raise err + err = ClientError("Gateway error") + setattr(err, "http_error", status_code) + raise err if url.endswith(f"/v1/project/push/cancel/{tx}"): return SimpleNamespace(msg="cancelled") return SimpleNamespace(msg="ok") @@ -52,12 +62,14 @@ def post(self, url, *args, **kwargs): project_path="u/p", changes={ "added": [], - "updated": [{ - "path": modified.name, - "size": file_size, - "diff": {"path": diff_path.name, "size": diff_size}, - "chunks": [1], - }], + "updated": [ + { + "path": modified.name, + "size": file_size, + "diff": {"path": diff_path.name, "size": diff_size}, + "chunks": [1], + } + ], "removed": [], }, transaction_id=tx, @@ -84,4 +96,4 @@ def post(self, url, *args, **kwargs): assert modified.name in text assert f"size={file_size}" in text assert f"diff_size={diff_size}" in text - assert ("changes=" in text) or ("changes=n/a" in text) \ No newline at end of file + assert ("changes=" in text) or ("changes=n/a" in text) From 8019b01bbecf4f761ddeb5c6d416e9c752bce004 Mon Sep 17 00:00:00 2001 From: DanChov Date: Thu, 11 Sep 2025 14:59:27 +0200 Subject: [PATCH 5/7] changes based on review: try-except should catch more geodiff errors and using real MerginProject in test_push_logging.py --- mergin/client_push.py | 17 ++++---- mergin/test/test_push_logging.py | 73 ++++++++++++++++---------------- 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index 3a099292..359cbab2 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -15,6 +15,13 @@ import tempfile import concurrent.futures import os +from pygeodiff import ( + GeoDiff, + GeoDiffLibError, + GeoDiffLibConflictError, + GeoDiffLibUnsupportedChangeError, + GeoDiffLibVersionError, +) from .common import UPLOAD_CHUNK_SIZE, ClientError from .merginproject import MerginProject @@ -326,16 +333,10 @@ def _geodiff_changes_count(mp: MerginProject, diff_rel_path: str): Never raises – diagnostics/logging must not fail. """ + diff_abs = mp.fpath_meta(diff_rel_path) try: - diff_abs = mp.fpath_meta(diff_rel_path) - except FileNotFoundError: - return None - - try: - from pygeodiff import GeoDiff, GeoDiffLibError - return GeoDiff().changes_count(diff_abs) - except GeoDiffLibError: + except (GeoDiffLibError, GeoDiffLibConflictError, GeoDiffLibUnsupportedChangeError, GeoDiffLibVersionError): return None diff --git a/mergin/test/test_push_logging.py b/mergin/test/test_push_logging.py index 1e6394e9..f2920f6a 100644 --- a/mergin/test/test_push_logging.py +++ b/mergin/test/test_push_logging.py @@ -2,62 +2,62 @@ from pathlib import Path import logging import tempfile +from unittest.mock import MagicMock import pytest from pygeodiff import GeoDiff from mergin.client_push import push_project_finalize, UploadJob from mergin.common import ClientError +from mergin.merginproject import MerginProject +from mergin.client import MerginClient @pytest.mark.parametrize("status_code", [502, 504]) def test_push_finalize_logs_on_5xx_real_diff(caplog, status_code, tmp_path): - # --- data --- + # test data data_dir = Path(__file__).resolve().parent / "test_data" base = data_dir / "base.gpkg" modified = data_dir / "inserted_1_A.gpkg" - assert base.exists() - assert modified.exists() + assert base.exists() and modified.exists() - diff_path = tmp_path / "base_to_inserted_1_A.diff" - GeoDiff().create_changeset(str(base), str(modified), str(diff_path)) - diff_size = diff_path.stat().st_size - file_size = modified.stat().st_size + # real MerginProject in temp dir + proj_dir = tmp_path / "proj" + meta_dir = proj_dir / ".mergin" + meta_dir.mkdir(parents=True) + mp = MerginProject(str(proj_dir)) - # --- logger --- + # route MP logs into pytest caplog logger = logging.getLogger("mergin_test") logger.setLevel(logging.DEBUG) logger.propagate = True caplog.set_level(logging.ERROR, logger="mergin_test") + mp.log = logger - # --- fake MP/MC len tam, kde treba --- - class MP: - def __init__(self, log): - self.log = log - - def update_metadata(self, *a, **k): - pass - - def apply_push_changes(self, *a, **k): - pass - - def fpath_meta(self, rel): - return str(diff_path) if rel == diff_path.name else rel + # generate a real diff into .mergin/ + diff_path = meta_dir / "base_to_inserted_1_A.diff" + GeoDiff().create_changeset(str(base), str(modified), str(diff_path)) + diff_rel = diff_path.name + diff_size = diff_path.stat().st_size + file_size = modified.stat().st_size + # mock MerginClient: only patch post(); simulate 5xx on finish tx = "tx-1" - class MC: - def post(self, url, *args, **kwargs): - if url.endswith(f"/v1/project/push/finish/{tx}"): - err = ClientError("Gateway error") - setattr(err, "http_error", status_code) - raise err - if url.endswith(f"/v1/project/push/cancel/{tx}"): - return SimpleNamespace(msg="cancelled") - return SimpleNamespace(msg="ok") + def mc_post(url, *args, **kwargs): + if url.endswith(f"/v1/project/push/finish/{tx}"): + err = ClientError("Gateway error") + setattr(err, "http_error", status_code) # emulate HTTP code on the exception + raise err + if url.endswith(f"/v1/project/push/cancel/{tx}"): + return SimpleNamespace(msg="cancelled") + return SimpleNamespace(msg="ok") + + mc = MagicMock(spec=MerginClient) + mc.post.side_effect = mc_post tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-test-") - # --- real UploadJob objekt --- + # build a real UploadJob that references the diff/file sizes job = UploadJob( project_path="u/p", changes={ @@ -66,25 +66,24 @@ def post(self, url, *args, **kwargs): { "path": modified.name, "size": file_size, - "diff": {"path": diff_path.name, "size": diff_size}, + "diff": {"path": diff_rel, "size": diff_size}, "chunks": [1], } ], "removed": [], }, transaction_id=tx, - mp=MP(logger), - mc=MC(), + mp=mp, + mc=mc, tmp_dir=tmp_dir, ) - # nastav to, čo finalize() očakáva job.total_size = 1234 job.transferred_size = 1234 - job.upload_queue_items = [1] # len pre log „Upload details“ + job.upload_queue_items = [1] job.executor = SimpleNamespace(shutdown=lambda wait=True: None) job.futures = [SimpleNamespace(done=lambda: True, exception=lambda: None, running=lambda: False)] - job.server_resp = {"version": "n/a"} # aj tak sa 5xx vyhodí skôr + job.server_resp = {"version": "n/a"} with pytest.raises(ClientError): push_project_finalize(job) From e65bf010beb6bfa4bb8e106619ee86dab16dbc91 Mon Sep 17 00:00:00 2001 From: DanChov Date: Fri, 12 Sep 2025 10:20:09 +0200 Subject: [PATCH 6/7] added FileNotFoundError --- mergin/client_push.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index 359cbab2..a78d9a17 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -336,7 +336,7 @@ def _geodiff_changes_count(mp: MerginProject, diff_rel_path: str): diff_abs = mp.fpath_meta(diff_rel_path) try: return GeoDiff().changes_count(diff_abs) - except (GeoDiffLibError, GeoDiffLibConflictError, GeoDiffLibUnsupportedChangeError, GeoDiffLibVersionError): + except (GeoDiffLibError, GeoDiffLibConflictError, GeoDiffLibUnsupportedChangeError, GeoDiffLibVersionError,FileNotFoundError): return None From f078c85576e13cc5964dbd5f37042af5cff629b6 Mon Sep 17 00:00:00 2001 From: DanChov Date: Fri, 12 Sep 2025 10:22:19 +0200 Subject: [PATCH 7/7] black --- mergin/client_push.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index a78d9a17..1e492082 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -336,7 +336,13 @@ def _geodiff_changes_count(mp: MerginProject, diff_rel_path: str): diff_abs = mp.fpath_meta(diff_rel_path) try: return GeoDiff().changes_count(diff_abs) - except (GeoDiffLibError, GeoDiffLibConflictError, GeoDiffLibUnsupportedChangeError, GeoDiffLibVersionError,FileNotFoundError): + except ( + GeoDiffLibError, + GeoDiffLibConflictError, + GeoDiffLibUnsupportedChangeError, + GeoDiffLibVersionError, + FileNotFoundError, + ): return None