address comments without progress bar @wonder-sk

Author: MarcelGeo

mergin/client.py

@@ -1519,6 +1519,9 @@ def sync_project(self, project_directory, progress_callback=None):
         2. Get local changes
         3. Push first change batch
         Repeat if there are more local changes.
+
+        :param project_directory: Project's directory
+        :param progress_callback: Optional callback function to report progress during push.
         """
         mp = MerginProject(project_directory)
         has_changes = True
@@ -1546,6 +1549,7 @@ def sync_project(self, project_directory, progress_callback=None):
                         last_size = current_size
                 push_project_finalize(job)
                 _, has_changes = get_push_changes_batch(self, mp)
+                server_conflict_attempts = 0
             except ClientError as e:
                 if e.is_retryable_sync() and server_conflict_attempts < PUSH_ATTEMPTS - 1:
                     # retry on conflict, e.g. when server has changes that we do not have yet
@@ -1556,5 +1560,3 @@ def sync_project(self, project_directory, progress_callback=None):
                     sleep(PUSH_ATTEMPT_WAIT)
                     continue
                 raise e
-            else:
-                server_conflict_attempts = 0

mergin/client_push.py

@@ -22,7 +22,7 @@
 import time
 from typing import List, Tuple, Optional, ByteString
 
-from .local_changes import LocalChange, LocalChanges
+from .local_changes import FileChange, LocalPojectChanges
 
 from .common import UPLOAD_CHUNK_ATTEMPT_WAIT, UPLOAD_CHUNK_ATTEMPTS, UPLOAD_CHUNK_SIZE, ClientError, ErrorCode
 from .merginproject import MerginProject
@@ -66,7 +66,7 @@ def __init__(
 
         self._request_headers = {"Content-Type": "application/octet-stream"}
 
-    def upload_chunk(self, data: ByteString, checksum: str):
+    def upload_chunk_v1_api(self, data: ByteString, checksum: str):
         """
         Uploads the chunk to the server.
         """
@@ -118,7 +118,7 @@ def upload_blocking(self):
                         self.upload_chunk_v2_api(data, checksum_str)
                     else:
                         # use v1 API for uploading chunks
-                        self.upload_chunk(data, checksum_str)
+                        self.upload_chunk_v1_api(data, checksum_str)
                     break  # exit loop if upload was successful
                 except ClientError as e:
                     if attempt < UPLOAD_CHUNK_ATTEMPTS - 1:
@@ -133,10 +133,10 @@ class UploadJob:
     """Keeps all the important data about a pending upload job"""
 
     def __init__(
-        self, version: str, changes: LocalChanges, transaction_id: Optional[str], mp: MerginProject, mc, tmp_dir
+        self, version: str, changes: LocalPojectChanges, transaction_id: Optional[str], mp: MerginProject, mc, tmp_dir
     ):
         self.version = version
-        self.changes: LocalChanges = changes  # dictionary of local changes to the project
+        self.changes: LocalPojectChanges = changes  # dictionary of local changes to the project
         self.transaction_id = transaction_id  # ID of the transaction assigned by the server
         self.total_size = 0  # size of data to upload (in bytes)
         self.transferred_size = 0  # size of data already uploaded (in bytes)
@@ -160,8 +160,8 @@ def _submit_item_to_thread(self, item: UploadQueueItem):
         future = self.executor.submit(_do_upload, item, self)
         self.futures.append(future)
 
-    def add_items(self, items: List[UploadQueueItem]):
-        """Add multiple chunks to the upload queue"""
+    def start(self, items: List[UploadQueueItem]):
+        """Starting upload in background with multiple upload items (UploadQueueItem)"""
         self.total_size = sum(item.size for item in items)
         self.upload_queue_items = items
 
@@ -175,7 +175,10 @@ def add_items(self, items: List[UploadQueueItem]):
             self._submit_item_to_thread(item)
 
     def update_chunks_from_items(self):
-        """Update chunks in LocalChanges from the upload queue items."""
+        """
+        Update chunks in LocalChanges from the upload queue items.
+        Used just before finalizing the transaction to set the server_chunk_id in v2 API.
+        """
         self.changes.update_chunks([(item.chunk_id, item.server_chunk_id) for item in self.upload_queue_items])
 
 
@@ -188,7 +191,7 @@ def _do_upload(item: UploadQueueItem, job: UploadJob):
     job.transferred_size += item.size
 
 
-def create_upload_chunks(mc, mp: MerginProject, local_changes: List[LocalChange]) -> List[UploadQueueItem]:
+def create_upload_chunks(mc, mp: MerginProject, local_changes: List[FileChange]) -> List[UploadQueueItem]:
     """
     Create a list of UploadQueueItem objects from the changes dictionary and calculate total size of files.
     This is used to prepare the upload queue for the upload job.
@@ -221,7 +224,7 @@ def create_upload_chunks(mc, mp: MerginProject, local_changes: List[LocalChange]
 
 
 def create_upload_job(
-    mc, mp: MerginProject, changes: LocalChanges, tmp_dir: tempfile.TemporaryDirectory
+    mc, mp: MerginProject, changes: LocalPojectChanges, tmp_dir: tempfile.TemporaryDirectory
 ) -> Optional[UploadJob]:
     """
     Prepare transaction and create an upload job for the project using the v1 API.
@@ -261,18 +264,20 @@ def create_upload_job(
     if not upload_changes:
         mp.log.info("not uploading any files")
         if push_start_resp:
+            # This is related just to v1 API
             job.server_resp = push_start_resp
         push_project_finalize(job)
         return  # all done - no pending job
 
     if transaction_id:
+        # This is related just to v1 API
         mp.log.info(f"got transaction ID {transaction_id}")
 
     # prepare file chunks for upload
     upload_queue_items = create_upload_chunks(mc, mp, upload_changes)
 
     mp.log.info(f"Starting upload chunks for project {project_id}")
-    job.add_items(upload_queue_items)
+    job.start(upload_queue_items)
     return job
 
 
@@ -312,10 +317,10 @@ def push_project_async(mc, directory) -> Optional[UploadJob]:
         if mp.is_versioned_file(f["path"]):
             mp.copy_versioned_file_for_upload(f, tmp_dir.name)
 
-    local_changes = LocalChanges(
-        added=[LocalChange(**change) for change in changes["added"]],
-        updated=[LocalChange(**change) for change in changes["updated"]],
-        removed=[LocalChange(**change) for change in changes["removed"]],
+    local_changes = LocalPojectChanges(
+        added=[FileChange(**change) for change in changes["added"]],
+        updated=[FileChange(**change) for change in changes["updated"]],
+        removed=[FileChange(**change) for change in changes["removed"]],
     )
     job = create_upload_job(mc, mp, local_changes, tmp_dir)
     return job
@@ -449,6 +454,7 @@ def push_project_cancel(job: UploadJob):
 
     job.executor.shutdown(wait=True)
     if not job.transaction_id:
+        # If not v2 api endpoint with transaction, nothing to cancel on server
         job.mp.log.info("--- push cancelled")
         return
     try:

mergin/editor.py

@@ -64,7 +64,7 @@ def prevent_conflicted_copy(path: str, mc, project_role: str) -> bool:
     Args:
         path (str): The file path to check.
         mc: The Mergin client object.
-        project_info (dict): Information about the Mergin project from server.
+        project_role: Current project role.
 
     Returns:
         bool: True if the file path should be prevented from ceating conflicted copy, False otherwise.

mergin/local_changes.py

@@ -4,17 +4,28 @@
 
 
 @dataclass
-class BaseLocalChange:
+class FileDiffChange:
     path: str
     checksum: str
     size: int
     mtime: datetime
 
 
 @dataclass
-class LocalChange(BaseLocalChange):
+class FileChange:
+    # path to the file relative to the project root
+    path: str
+    # file checksum
+    checksum: str
+    # file size in bytes
+    size: int
+    # file modification time
+    mtime: datetime
+    # original file checksum used for compairison
     origin_checksum: Optional[str] = None
+    # list of chunk ids that make up this file
     chunks: List[str] = field(default_factory=list)
+    # optional diff information for geopackage files with geodiff metadata
     diff: Optional[dict] = None
     upload_file: Optional[str] = None
     # some functions (MerginProject.compare_file_sets) are adding version to the change from project info
@@ -24,9 +35,9 @@ class LocalChange(BaseLocalChange):
     # some functions (MerginProject.compare_file_sets) are adding location dict to the change from project info
     location: Optional[str] = None
 
-    def get_diff(self) -> Optional[BaseLocalChange]:
+    def get_diff(self) -> Optional[FileDiffChange]:
         if self.diff:
-            return BaseLocalChange(
+            return FileDiffChange(
                 path=self.diff.get("path", ""),
                 checksum=self.diff.get("checksum", ""),
                 size=self.diff.get("size", 0),
@@ -50,10 +61,10 @@ def to_server_data(self) -> dict:
 
 
 @dataclass
-class LocalChanges:
-    added: List[LocalChange] = field(default_factory=list)
-    updated: List[LocalChange] = field(default_factory=list)
-    removed: List[LocalChange] = field(default_factory=list)
+class LocalPojectChanges:
+    added: List[FileChange] = field(default_factory=list)
+    updated: List[FileChange] = field(default_factory=list)
+    removed: List[FileChange] = field(default_factory=list)
 
     def to_server_payload(self) -> dict:
         return {
@@ -62,7 +73,7 @@ def to_server_payload(self) -> dict:
             "removed": [change.to_server_data() for change in self.removed],
         }
 
-    def get_upload_changes(self) -> List[LocalChange]:
+    def get_upload_changes(self) -> List[FileChange]:
         """
         Get all changes that need to be uploaded.
         This includes added and updated files.
@@ -76,9 +87,7 @@ def _map_unique_chunks(self, change_chunks: List[str], server_chunks: List[Tuple
         mapped = []
         seen = set()
         for chunk in change_chunks:
-            for server_chunk in server_chunks:
-                chunk_id = server_chunk[0]
-                server_chunk_id = server_chunk[1]
+            for chunk_id, server_chunk_id in server_chunks:
                 if chunk_id == chunk and server_chunk_id not in seen:
                     mapped.append(server_chunk_id)
                     seen.add(server_chunk_id)

mergin/test/test_local_changes.py

@@ -1,6 +1,6 @@
 from datetime import datetime
 
-from ..local_changes import LocalChange, LocalChanges
+from ..local_changes import FileChange, LocalPojectChanges
 
 
 def test_local_changes_from_dict():
@@ -31,11 +31,11 @@ def test_local_changes_from_dict():
     }
 
     # Convert dictionary to LocalChanges
-    added = [LocalChange(**file) for file in changes_dict["added"]]
-    updated = [LocalChange(**file) for file in changes_dict["updated"]]
-    removed = [LocalChange(**file) for file in changes_dict["removed"]]
+    added = [FileChange(**file) for file in changes_dict["added"]]
+    updated = [FileChange(**file) for file in changes_dict["updated"]]
+    removed = [FileChange(**file) for file in changes_dict["removed"]]
 
-    local_changes = LocalChanges(added=added, updated=updated, removed=removed)
+    local_changes = LocalPojectChanges(added=added, updated=updated, removed=removed)
 
     # Assertions
     assert len(local_changes.added) == 1
@@ -51,7 +51,7 @@ def test_local_changes_from_dict():
 
 def test_local_change_get_diff():
     """Test the get_diff method of LocalChange."""
-    local_change = LocalChange(
+    local_change = FileChange(
         path="file1.txt",
         checksum="abc123",
         size=1024,
@@ -68,11 +68,11 @@ def test_local_change_get_diff():
 
 def test_local_changes_to_server_payload():
     """Test the to_server_payload method of LocalChanges."""
-    added = [LocalChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now())]
-    updated = [LocalChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now())]
-    removed = [LocalChange(path="file3.txt", checksum="lmn456", size=512, mtime=datetime.now())]
+    added = [FileChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now())]
+    updated = [FileChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now())]
+    removed = [FileChange(path="file3.txt", checksum="lmn456", size=512, mtime=datetime.now())]
 
-    local_changes = LocalChanges(added=added, updated=updated, removed=removed)
+    local_changes = LocalPojectChanges(added=added, updated=updated, removed=removed)
     server_request = local_changes.to_server_payload()
 
     assert "added" in server_request
@@ -86,12 +86,12 @@ def test_local_changes_to_server_payload():
 def test_local_changes_update_chunks():
     """Test the update_chunks method of LocalChanges."""
     added = [
-        LocalChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now(), chunks=["abc123"]),
-        LocalChange(path="file2.txt", checksum="abc123", size=1024, mtime=datetime.now(), chunks=["abc123"]),
+        FileChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now(), chunks=["abc123"]),
+        FileChange(path="file2.txt", checksum="abc123", size=1024, mtime=datetime.now(), chunks=["abc123"]),
     ]
-    updated = [LocalChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now(), chunks=["xyz789"])]
+    updated = [FileChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now(), chunks=["xyz789"])]
 
-    local_changes = LocalChanges(added=added, updated=updated)
+    local_changes = LocalPojectChanges(added=added, updated=updated)
     chunks = [("abc123", "chunk1"), ("abc123", "chunk1"), ("xyz789", "chunk2")]
 
     local_changes.update_chunks(chunks)
@@ -104,12 +104,12 @@ def test_local_changes_update_chunks():
 def test_local_changes_get_upload_changes():
     """Test the get_upload_changes method of LocalChanges."""
     # Create sample LocalChange instances
-    added = [LocalChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now())]
-    updated = [LocalChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now())]
-    removed = [LocalChange(path="file3.txt", checksum="lmn456", size=512, mtime=datetime.now())]
+    added = [FileChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now())]
+    updated = [FileChange(path="file2.txt", checksum="xyz789", size=2048, mtime=datetime.now())]
+    removed = [FileChange(path="file3.txt", checksum="lmn456", size=512, mtime=datetime.now())]
 
     # Initialize LocalChanges with added, updated, and removed changes
-    local_changes = LocalChanges(added=added, updated=updated, removed=removed)
+    local_changes = LocalPojectChanges(added=added, updated=updated, removed=removed)
 
     # Call get_upload_changes
     upload_changes = local_changes.get_upload_changes()