bug: push large projects file by file when projects/update returns 413

lean/components/api/file_client.py

@@ -11,7 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import List
+from typing import List, Optional
 
 from lean.components.api.api_client import *
 from lean.models.api import QCFullFile, QCMinimalFile
@@ -53,39 +53,49 @@ def get_all(self, project_id: int) -> List[QCFullFile]:
 
         return [QCFullFile(**file) for file in data["files"]]
 
-    def create(self, project_id: int, file_name: str, content: str) -> None:
+    def create(self, project_id: int, file_name: str, content: str, code_source_id: Optional[str] = None) -> None:
         """Creates a new file.
 
         :param project_id: the id of the project to create a file for
         :param file_name: the name of the file to create
         :param content: the content of the file to create
+        :param code_source_id: the source of the code changes (e.g., "cli")
         """
-        data = self._api.post("files/create", {
+        self._api.post("files/create", self._with_code_source_id({
             "projectId": project_id,
             "name": file_name,
             "content": content
-        })
+        }, code_source_id))
 
-    def update(self, project_id: int, file_name: str, content: str) -> None:
+    def update(self, project_id: int, file_name: str, content: str, code_source_id: Optional[str] = None) -> None:
         """Updates an existing file.
 
         :param project_id: the id of the project to update a file in
         :param file_name: the name of the file to update
         :param content: the new content of the file
+        :param code_source_id: the source of the code changes (e.g., "cli")
         """
-        data = self._api.post("files/update", {
+        self._api.post("files/update", self._with_code_source_id({
             "projectId": project_id,
             "name": file_name,
             "content": content
-        })
+        }, code_source_id))
 
-    def delete(self, project_id: int, file_name: str) -> None:
+    def delete(self, project_id: int, file_name: str, code_source_id: Optional[str] = None) -> None:
         """Deletes an existing file.
 
         :param project_id: the id of the project the file belongs to
         :param file_name: the name of the file to delete
+        :param code_source_id: the source of the code changes (e.g., "cli")
         """
-        self._api.post("files/delete", {
+        self._api.post("files/delete", self._with_code_source_id({
             "projectId": project_id,
             "name": file_name
-        })
+        }, code_source_id))
+
+    @staticmethod
+    def _with_code_source_id(request_parameters: dict, code_source_id: Optional[str]) -> dict:
+        # The API only checks the project's collaboration lock when codeSourceId is sent
+        if code_source_id is not None:
+            request_parameters["codeSourceId"] = code_source_id
+        return request_parameters

lean/components/cloud/push_manager.py

@@ -12,7 +12,8 @@
 # limitations under the License.
 
 from pathlib import Path
-from typing import List, Dict, Optional
+from functools import partial
+from typing import Any, List, Dict, Optional
 
 from lean.components.api.api_client import APIClient
 from lean.components.config.project_config_manager import ProjectConfigManager
@@ -22,6 +23,7 @@
 from lean.models.api import QCLanguage, QCProject
 from lean.models.utils import LeanLibraryReference
 from lean.models.encryption import ActionType
+from lean.models.errors import PartialPushError, RequestFailedError
 
 class PushManager:
     """The PushManager class is responsible for synchronizing local projects to the cloud."""
@@ -69,6 +71,7 @@ def push_projects(self, projects_to_push: List[Path], associated_libraries_to_pu
             return
 
         organization_id = self._organization_manager.try_get_working_organization_id()
+        partly_pushed_projects = []
 
         for index, path in enumerate(all_projects_to_push, start=1):
 
@@ -85,6 +88,14 @@ def push_projects(self, projects_to_push: List[Path], associated_libraries_to_pu
                 self._logger.warn(f"Cannot push '{relative_path}': {ex}")
                 if "write permission" in str(ex).lower():
                     self._logger.info("Please pull any required changes and push with --force")
+                if isinstance(ex, PartialPushError):
+                    partly_pushed_projects.append(relative_path.as_posix())
+
+        # A partly pushed project must not be compiled, backtested or deployed by the command that pushed it
+        if len(partly_pushed_projects) > 0:
+            raise PartialPushError(f"Some projects were only partly pushed: "
+                                   f"{', '.join(repr(name) for name in partly_pushed_projects)}. "
+                                   f"Push again after fixing the errors above")
 
     def _get_local_libraries_cloud_ids(self, project_dir: Path) -> List[int]:
         project_config = self._project_config_manager.get_project_config(project_dir)
@@ -259,7 +270,12 @@ def _push_metadata(self, project: Path, cloud_project: QCProject, encryption_act
         if not force:
             update_args["code_source_id"] = "cli"
         if update_args != {}:
-            self._api_client.projects.update(cloud_project.projectId, **update_args)
+            try:
+                self._api_client.projects.update(cloud_project.projectId, **update_args)
+            except RequestFailedError as error:
+                if error.response.status_code != 413:
+                    raise
+                self._push_one_by_one(cloud_project, update_args)
 
             if "encryption_key" in update_args:
                 del update_args["encryption_key"]
@@ -274,6 +290,56 @@ def _push_metadata(self, project: Path, cloud_project: QCProject, encryption_act
 
             self._logger.info(f"Successfully updated {updated_keys_str} for '{cloud_project.name}'")
 
+    def _push_one_by_one(self, cloud_project: QCProject, update_args: Dict[str, Any]) -> None:
+        """Pushes a project that is too large for a single projects/update request, with one request per change.
+
+        The files go first and the other metadata last. The API checks the collaboration lock on files/* requests
+        that carry codeSourceId, but not on a projects/update without files, so a lock held by someone else stops
+        the push before any file is written.
+
+        :param cloud_project: the cloud project to push to
+        :param update_args: the arguments of the projects/update request that was too large
+        """
+        project_id = cloud_project.projectId
+        code_source_id = update_args.get("code_source_id")
+        local_files = update_args["files"]
+        local_file_names = {file["name"] for file in local_files}
+        self._logger.info(f"'{cloud_project.name}' is too large to push in one request, pushing its changed files one by one")
+        cloud_files = {file.name: file for file in self._api_client.files.get_all(project_id)}
+
+        # Deletes go first, so a rename in a project at its file quota doesn't go over the quota
+        requests = [(f"deleting '{cloud_file.name}'",
+                     partial(self._api_client.files.delete, project_id, cloud_file.name, code_source_id=code_source_id))
+                    for cloud_file in cloud_files.values()
+                    if not cloud_file.isLibrary and cloud_file.name not in local_file_names]
+        for file in local_files:
+            cloud_file = cloud_files.get(file["name"])
+            if cloud_file is None:
+                requests.append((f"creating '{file['name']}'",
+                                 partial(self._api_client.files.create, project_id, file["name"], file["content"],
+                                         code_source_id=code_source_id)))
+            # Trailing whitespace can't change the code, leading whitespace can (Python indentation)
+            elif cloud_file.content.rstrip() != file["content"].rstrip():
+                requests.append((f"updating '{file['name']}'",
+                                 partial(self._api_client.files.update, project_id, file["name"], file["content"],
+                                         code_source_id=code_source_id)))
+        requests.append(("updating the project details",
+                         partial(self._api_client.projects.update, project_id,
+                                 **{key: value for key, value in update_args.items() if key != "files"})))
+
+        for index, (description, request) in enumerate(requests):
+            try:
+                request()
+            except Exception as error:
+                if index == 0:
+                    raise RuntimeError(f"{description} failed, nothing was pushed: {error}") from error
+                if index == len(requests) - 1:
+                    raise PartialPushError(f"All files of '{cloud_project.name}' were pushed, but updating the project "
+                                           f"details failed. Fix the problem and push again. Error: {error}") from error
+                raise PartialPushError(f"Only part of '{cloud_project.name}' was pushed: {description} failed, so the "
+                                       f"cloud project now mixes old and new code. Fix the problem and push again. "
+                                       f"Error: {error}") from error
+
     def _get_cloud_project(self, project_id: int, organization_id: str) -> QCProject:
         project = next(iter(p for p in self._cloud_projects if p.projectId == project_id), None)
         if project is None:

lean/models/errors.py

@@ -34,6 +34,10 @@ def __init__(self, response, message: Optional[str] = None) -> None:
         self.response = response
 
 
+class PartialPushError(Exception):
+    """A PartialPushError indicates that a push stopped after some of the project's changes were written to the cloud."""
+
+
 class MoreInfoError(Exception):
     """An error which consists of a message and a link to documentation with more information."""
 

tests/components/api/test_file_client.py

@@ -0,0 +1,36 @@
+# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals.
+# Lean CLI v1.0. Copyright 2021 QuantConnect Corporation.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Optional
+from unittest import mock
+
+import pytest
+
+from lean.components.api.file_client import FileClient
+
+
+@pytest.mark.parametrize("method, args, endpoint, expected_parameters", [
+    ("create", (1, "main.py", "content"), "files/create", {"projectId": 1, "name": "main.py", "content": "content"}),
+    ("update", (1, "main.py", "content"), "files/update", {"projectId": 1, "name": "main.py", "content": "content"}),
+    ("delete", (1, "main.py"), "files/delete", {"projectId": 1, "name": "main.py"})
+])
+@pytest.mark.parametrize("code_source_id", [None, "cli"])
+def test_file_client_sends_code_source_id_only_when_given(method: str, args: tuple, endpoint: str,
+                                                          expected_parameters: dict, code_source_id: Optional[str]) -> None:
+    api_client = mock.Mock()
+
+    getattr(FileClient(api_client), method)(*args, code_source_id=code_source_id)
+
+    if code_source_id is not None:
+        expected_parameters = {**expected_parameters, "codeSourceId": code_source_id}
+    api_client.post.assert_called_once_with(endpoint, expected_parameters)

tests/components/cloud/test_cloud_project_manager.py

@@ -14,7 +14,10 @@
 from pathlib import Path
 from unittest import mock
 
+import pytest
+
 from lean.container import container
+from lean.models.errors import PartialPushError
 from tests.test_helpers import create_api_project, create_fake_lean_cli_directory
 from tests.conftest import initialize_container
 
@@ -49,3 +52,19 @@ def test_get_cloud_project_pushing_new_project():
 
     api_client.projects.get.assert_called_with(cloud_project.projectId, "abc")
     push_manager.push_projects.assert_called_once_with([Path.cwd() / "Python Project"])
+
+
+def test_get_cloud_project_does_not_return_a_partly_pushed_project():
+    create_fake_lean_cli_directory()
+
+    api_client = mock.Mock()
+    push_manager = mock.Mock()
+    push_manager.push_projects = mock.Mock(side_effect=PartialPushError("Some projects were only partly pushed"))
+
+    initialize_container(api_client_to_use=api_client, push_manager_to_use=push_manager)
+
+    # Commands like "lean cloud backtest --push" must not run on a mix of old and new cloud code
+    with pytest.raises(PartialPushError):
+        container.cloud_project_manager.get_cloud_project("Python Project", push=True)
+
+    api_client.projects.get.assert_not_called()