Cleanup and docstrings

Author: harminius

README.md

@@ -85,7 +85,7 @@ Commands:
   show-file-history    Display information about a single version of a...
   show-version         Display information about a single version of a...
   status               Show all changes in project files - upstream and...
-  sync                 Syncronize the project.
+  sync                 Synchronize the project.
 ```
 
 ### Examples

mergin/cli.py

@@ -411,23 +411,13 @@ def sync(ctx):
         return
     directory = os.getcwd()
 
-    size = total_upload_size(directory)
-    if size == 0:
-        click.secho("Already up to date.", fg="green")
-        return
-
     try:
-        uploaded_so_far = 0
-
-        with click.progressbar(length=size, label="Uploading changes") as bar:
-            # updates the progress bar
-            def on_progress(last, now):
-                nonlocal uploaded_so_far
-                delta = now - last
-                uploaded_so_far += delta
-                bar.update(delta)
+        size = total_upload_size(directory)
+        with click.progressbar(length=size, label="Syncing") as bar:
+            def on_progress(increment):
+                bar.update(increment)
 
-            # run pull & push cycles
+            # run pull & push cycles until there are no local changes
             mc.sync_project_with_callback(directory, progress_callback=on_progress)
 
         click.secho("Sync complete.", fg="green")
@@ -439,8 +429,10 @@ def on_progress(last, now):
         return
     except KeyboardInterrupt:
         click.secho("Cancelling...")
-        push_project_cancel(mc.upload_job)
-        pull_project_cancel(mc.download_job)
+        if mc.pull_job:
+            pull_project_cancel(mc.pull_job)
+        if mc.push_job:
+            push_project_cancel(mc.push_job)
     except Exception as e:
         _print_unhandled_exception()
 

mergin/client.py

@@ -33,13 +33,12 @@
     download_project_finalize,
     download_project_wait,
     download_diffs_finalize,
-    pull_project_is_running,
     pull_project_async,
     pull_project_wait,
     pull_project_finalize
 )
 from .client_push import push_project_wait, push_project_finalize, push_project_async, push_project_is_running, \
-    ChangesHandler
+    ChangesHandler, get_change_batch
 from .utils import DateTimeEncoder, get_versions_with_file_changes, int_version, is_version_acceptable
 from .version import __version__
 
@@ -936,45 +935,48 @@ def sync_project(self, project_dir):
         has_more_changes = True
         while has_more_changes:
             self.pull_job = pull_project_async(self, project_dir)
-            pull_project_wait(self.pull_job)
-            pull_project_finalize(self.pull_job)
+            if self.pull_job is not None:
+                pull_project_wait(self.pull_job)
+                pull_project_finalize(self.pull_job)
 
-            changes_handler = ChangesHandler(self, project_dir)
-            changes_batch, has_more_changes = changes_handler.get_change_batch()
+            changes_batch, has_more_changes = get_change_batch(self, project_dir)
+            if not changes_batch:  # no changes to upload, quit sync
+                return
 
-            self.push_job = push_project_async(project_dir, changes_batch)
-            push_project_wait(self.push_job)
-            push_project_finalize(self.push_job)
+            self.push_job = push_project_async(self, project_dir, changes_batch)
+            if self.push_job:
+                push_project_wait(self.push_job)
+                push_project_finalize(self.push_job)
 
-    def sync_project_with_callback(self, project_dir, progress_callback=None, sleep_time=100):
+    def sync_project_with_callback(self, project_dir, progress_callback=None, sleep_time=0.1):
         """
         Syncs project while sending push progress info as callback.
         Sync is done in this loop:
-            Pending changes? -> Pull -> Push change batch -> repeat
+            Pending changes? -> Pull -> Get changes batch -> Push the changes -> repeat
+
+        :param progress_callback: updates the progress bar in CLI, on_progress(increment)
+        :param sleep_time: sleep time between calling the callback function
         """
         has_more_changes = True
         while has_more_changes:
-            changes_handler = ChangesHandler(self, project_dir)
-
             self.pull_job = pull_project_async(self, project_dir)
-            if self.pull_job is None:
-                return
-            pull_project_wait(self.pull_job)
-            pull_project_finalize(self.pull_job)
-
-            changes_batch, has_more_changes = changes_handler.get_change_batch()
+            if self.pull_job is not None:
+                pull_project_wait(self.pull_job)
+                pull_project_finalize(self.pull_job)
 
-            self.push_job = push_project_async(self, project_dir, changes_batch)
-            if not self.push_job:
+            changes_batch, has_more_changes = get_change_batch(self, project_dir)
+            if not changes_batch:  # no changes to upload, quit sync
                 return
 
-            last = 0
-            while push_project_is_running(self.push_job):
-                sleep(sleep_time)
-                now = self.push_job.transferred_size
-                progress_callback(last, now)
-                last = now
-            push_project_finalize(self.push_job)
+            self.push_job = push_project_async(self, project_dir, changes_batch)
+            if self.push_job:
+                last = 0
+                while push_project_is_running(self.push_job):
+                    sleep(sleep_time)
+                    now = self.push_job.transferred_size
+                    progress_callback(now - last)  # update progressbar with transferred size increment
+                    last = now
+                push_project_finalize(self.push_job)
 
 
     def clone_project(self, source_project_path, cloned_project_name, cloned_project_namespace=None):

mergin/client_push.py

@@ -132,8 +132,8 @@ def _split_by_type(self, changes: Dict[str, List[dict]]) -> List[Dict[str, List[
         1. Blocking: updated/removed and added files that are blocking
         2. Non-blocking: added files that are not blocking
         """
-        blocking_changes = {"added": [], "updated": [], "removed": [], "renamed": []}
-        non_blocking_changes = {"added": [], "updated": [], "removed": [], "renamed": []}
+        blocking_changes = {"added": [], "updated": [], "removed": []}
+        non_blocking_changes = {"added": [], "updated": [], "removed": []}
 
         for f in changes.get("added", []):
             if self.is_blocking_file(f):
@@ -164,18 +164,26 @@ def split(self) -> List[Dict[str, List[dict]]]:
         # TODO: apply limits; changes = self._limit_by_file_count(changes)
         return changes_list
 
-    def get_change_batch(self) -> Tuple[Optional[Dict[str, List[dict]]], bool]:
-        """
-        Return the next changes dictionary and flag if there are more changes
-        """
-        changes_list = self.split()
-        if not changes_list:
-            return None, False
-        return changes_list[0], len(changes_list) > 1
+def get_change_batch(mc, project_dir) -> Tuple[Optional[Dict[str, List[dict]]], bool]:
+    """
+    Return the next changes dictionary and flag if there are more changes (to be uploaded in the next upload job)
+    """
+    changes_list = ChangesHandler(mc, project_dir).split()
+    if not changes_list:
+        return None, False
+    non_empty_length = sum(any(v for v in d.values()) for d in changes_list)
+    return changes_list[0], non_empty_length > 1
 
 
 def push_project_async(mc, directory, change_batch=None) -> Optional[UploadJob]:
-    """Starts push of a change of a project and returns pending upload job"""
+    """
+    Starts push in background and returns pending upload job.
+    Pushes all project changes unless change_batch is provided.
+    When specific change is provided, initial version check is skipped (the pull has just been done).
+
+    :param change_batch: A dictionary of changes that was split to blocking and non-blocking.
+    Pushing only non-blocking changes results in non-exclusive upload which server allows to be concurrent.
+    """
 
     mp = MerginProject(directory)
     if mp.has_unfinished_pull():
@@ -187,30 +195,32 @@ def push_project_async(mc, directory, change_batch=None) -> Optional[UploadJob]:
     mp.log.info("--- version: " + mc.user_agent_info())
     mp.log.info(f"--- start push {project_path}")
 
-    try:
-        project_info = mc.project_info(project_path)
-    except ClientError as err:
-        mp.log.error("Error getting project info: " + str(err))
-        mp.log.info("--- push aborted")
-        raise
-    server_version = project_info["version"] if project_info["version"] else "v0"
-
-    mp.log.info(f"got project info: local version {local_version} / server version {server_version}")
-
-    username = mc.username()
-    # permissions field contains information about update, delete and upload privileges of the user
-    # on a specific project. This is more accurate information than "writernames" field, as it takes
-    # into account namespace privileges. So we have to check only "permissions", namely "upload" once
-    if not mc.has_writing_permissions(project_path):
-        mp.log.error(f"--- push {project_path} - username {username} does not have write access")
-        raise ClientError(f"You do not seem to have write access to the project (username '{username}')")
-
-    if local_version != server_version:
-        mp.log.error(f"--- push {project_path} - not up to date (local {local_version} vs server {server_version})")
-        raise ClientError(
-            "There is a new version of the project on the server. Please update your local copy."
-            + f"\n\nLocal version: {local_version}\nServer version: {server_version}"
-        )
+    # if we have specific change to push we don't need version check
+    if not change_batch:
+        try:
+            project_info = mc.project_info(project_path)
+        except ClientError as err:
+            mp.log.error("Error getting project info: " + str(err))
+            mp.log.info("--- push aborted")
+            raise
+        server_version = project_info["version"] if project_info["version"] else "v0"
+
+        mp.log.info(f"got project info: local version {local_version} / server version {server_version}")
+
+        username = mc.username()
+        # permissions field contains information about update, delete and upload privileges of the user
+        # on a specific project. This is more accurate information than "writernames" field, as it takes
+        # into account namespace privileges. So we have to check only "permissions", namely "upload" once
+        if not mc.has_writing_permissions(project_path):
+            mp.log.error(f"--- push {project_path} - username {username} does not have write access")
+            raise ClientError(f"You do not seem to have write access to the project (username '{username}')")
+
+        if local_version != server_version:
+            mp.log.error(f"--- push {project_path} - not up to date (local {local_version} vs server {server_version})")
+            raise ClientError(
+                "There is a new version of the project on the server. Please update your local copy."
+                + f"\n\nLocal version: {local_version}\nServer version: {server_version}"
+            )
 
     changes = change_batch or mp.get_push_changes()
     mp.log.debug("push change:\n" + pprint.pformat(changes))
@@ -288,16 +298,16 @@ def push_project_async(mc, directory, change_batch=None) -> Optional[UploadJob]:
 
         total_size += file_size
 
-        job.total_size = total_size
-        job.upload_queue_items = upload_queue_items
+    job.total_size = total_size
+    job.upload_queue_items = upload_queue_items
 
-        mp.log.info(f"will upload {len(upload_queue_items)} items with total size {total_size}")
+    mp.log.info(f"will upload {len(upload_queue_items)} items with total size {total_size}")
 
-        # start uploads in background
-        job.executor = concurrent.futures.ThreadPoolExecutor(max_workers=4)
-        for item in upload_queue_items:
-            future = job.executor.submit(_do_upload, item, job)
-            job.futures.append(future)
+    # start uploads in background
+    job.executor = concurrent.futures.ThreadPoolExecutor(max_workers=4)
+    for item in upload_queue_items:
+        future = job.executor.submit(_do_upload, item, job)
+        job.futures.append(future)
 
     return job
 
@@ -435,7 +445,9 @@ def total_upload_size(directory) -> int:
     mp = MerginProject(directory)
     changes = mp.get_push_changes()
     files = changes.get("added", []) + changes.get("updated", [])
-    return sum(
+    size = sum(
         f.get("diff", {}).get("size", f.get("size", 0))
         for f in files
-    )
+    )
+    mp.log.info(f"Upload size of all files is {size}")
+    return size

mergin/merginproject.py

@@ -464,10 +464,6 @@ def get_push_changes(self) -> Dict[str, List[dict]]:
                 pass
 
         changes["updated"] = [f for f in changes["updated"] if f not in not_updated]
-        if changes:
-            self.log.debug(f"All local changes:\n" + pprint.pformat(changes))
-        else:
-            self.log.debug("No local changes. Nothing to upload.")
         return changes
 
     def copy_versioned_file_for_upload(self, f, tmp_dir):