From 5ed7de7519ecf78de97561ab558028548c04bc9d Mon Sep 17 00:00:00 2001 From: Bruno Grego Date: Fri, 21 Nov 2025 17:59:41 +0000 Subject: [PATCH 1/5] Prevent upload of empty zip --- .../task_runner/task_request_handler.py | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/task-runner/task_runner/task_request_handler.py b/task-runner/task_runner/task_request_handler.py index a88a268b..ff160d15 100644 --- a/task-runner/task_runner/task_request_handler.py +++ b/task-runner/task_runner/task_request_handler.py @@ -197,17 +197,26 @@ def save_output( new_task_status=None, force=False, output_filename=None, - ): - _ = self._pack_output(output_filename=output_filename) - self._publish_event(events.TaskOutputUploaded( - id=self.task_id, - machine_id=self.task_runner_uuid, - new_status=new_task_status), - force=force) + ) -> bool: + output_size_bytes = self._pack_output(output_filename=output_filename) + + if output_size_bytes == 0: + return False + + self._publish_event( + events.TaskOutputUploaded( + id=self.task_id, + machine_id=self.task_runner_uuid, + new_status=new_task_status, + ), + force=force, + ) + # Remove the request JSON file to prevent multiple uploads # after a successful task data upload if os.path.exists(self.request_path): os.remove(self.request_path) + return True def is_task_running(self) -> bool: @@ -615,21 +624,33 @@ def _execute_request( return exit_code, exit_reason def _pack_output(self, output_filename: Optional[str] = None) -> int: - """Compress outputs and store them in the shared drive.""" + """Compress and upload outputs. If the output size is 0 or not + determined (which may happen during unusual conditions) the upload is + aborted.""" + output_size_bytes = 0 + if self.task_workdir is None: logging.error("Working directory not found.") - return + return output_size_bytes output_dir = os.path.join(self.task_workdir, utils.OUTPUT_DIR) if not os.path.exists(output_dir): logging.error("Output directory not found: %s", output_dir) - return + return output_size_bytes output_size_bytes = files.get_dir_size(output_dir) logging.info("Output size: %s bytes", output_size_bytes) - if output_size_bytes is not None: - self._post_task_metric(utils.OUTPUT_SIZE, output_size_bytes) + if output_size_bytes is None: + logging.error( + "Failed to determine size for output directory: %s", output_dir + ) + output_size_bytes == 0 + + if output_size_bytes == 0: + return output_size_bytes + + self._post_task_metric(utils.OUTPUT_SIZE, output_size_bytes) output_total_files = files.get_dir_total_files(output_dir) logging.info("Output total files: %s", output_total_files) From 2a5dfdbe66969bbdcc6581fa3258dceb9a22d872 Mon Sep 17 00:00:00 2001 From: Bruno Grego Date: Fri, 21 Nov 2025 18:24:24 +0000 Subject: [PATCH 2/5] Fix formatting and tests --- task-runner/task_runner/task_request_handler.py | 5 ++--- task-runner/tests/unit/test_task_request_handler.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/task-runner/task_runner/task_request_handler.py b/task-runner/task_runner/task_request_handler.py index ff160d15..9f81d0e7 100644 --- a/task-runner/task_runner/task_request_handler.py +++ b/task-runner/task_runner/task_request_handler.py @@ -642,9 +642,8 @@ def _pack_output(self, output_filename: Optional[str] = None) -> int: logging.info("Output size: %s bytes", output_size_bytes) if output_size_bytes is None: - logging.error( - "Failed to determine size for output directory: %s", output_dir - ) + logging.error("Failed to determine size for output directory: %s", + output_dir) output_size_bytes == 0 if output_size_bytes == 0: diff --git a/task-runner/tests/unit/test_task_request_handler.py b/task-runner/tests/unit/test_task_request_handler.py index 57117d43..269e88fd 100644 --- a/task-runner/tests/unit/test_task_request_handler.py +++ b/task-runner/tests/unit/test_task_request_handler.py @@ -137,7 +137,7 @@ def fixture_task_request_handler( "task_runner.api_methods_config.get_executer") as get_executer_mock: with mock.patch( "task_runner.utils.files.get_dir_size") as get_dir_size_mock: - get_dir_size_mock.return_value = 0 + get_dir_size_mock.return_value = 1000 get_executer_mock.return_value = MockExecuter yield handler From 68f6b28e2390e2d5350c3280e33b6843e9578aad Mon Sep 17 00:00:00 2001 From: Bruno Grego Date: Mon, 24 Nov 2025 09:40:50 +0000 Subject: [PATCH 3/5] Fix tests --- Makefile | 5 ++++- task-runner/requirements.txt | 1 + task-runner/tests/unit/test_task_request_handler.py | 8 +++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 29d587ec..4954ff83 100644 --- a/Makefile +++ b/Makefile @@ -81,5 +81,8 @@ lint-fix: format: yapf . --in-place --recursive --parallel --exclude=third_party +test-lite: + $(DOCKER_COMPOSE_COMMAND_TASK_RUNNER_LITE) run --build --rm task-runner-lite \ + pytest -q -style: format lint-fix \ No newline at end of file +style: format lint-fix diff --git a/task-runner/requirements.txt b/task-runner/requirements.txt index 4eb5bcce..d0f0eff9 100644 --- a/task-runner/requirements.txt +++ b/task-runner/requirements.txt @@ -9,3 +9,4 @@ pysocks gputil pydantic~=2.11.7 tenacity +pytest diff --git a/task-runner/tests/unit/test_task_request_handler.py b/task-runner/tests/unit/test_task_request_handler.py index 269e88fd..d3ec113f 100644 --- a/task-runner/tests/unit/test_task_request_handler.py +++ b/task-runner/tests/unit/test_task_request_handler.py @@ -134,11 +134,9 @@ def fixture_task_request_handler( ) with mock.patch( - "task_runner.api_methods_config.get_executer") as get_executer_mock: - with mock.patch( - "task_runner.utils.files.get_dir_size") as get_dir_size_mock: - get_dir_size_mock.return_value = 1000 - get_executer_mock.return_value = MockExecuter + "task_runner.api_methods_config.get_executer", return_value=MockExecuter + ): + with mock.patch.object(handler, "_pack_output", return_value=1000): yield handler From 676ecba9b6a3549feec64387e26ce2666892e81e Mon Sep 17 00:00:00 2001 From: Bruno Grego Date: Mon, 24 Nov 2025 09:47:22 +0000 Subject: [PATCH 4/5] Fix formatting --- task-runner/tests/unit/test_task_request_handler.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/task-runner/tests/unit/test_task_request_handler.py b/task-runner/tests/unit/test_task_request_handler.py index d3ec113f..3f60c404 100644 --- a/task-runner/tests/unit/test_task_request_handler.py +++ b/task-runner/tests/unit/test_task_request_handler.py @@ -133,9 +133,8 @@ def fixture_task_request_handler( file_manager=mock.MagicMock(), ) - with mock.patch( - "task_runner.api_methods_config.get_executer", return_value=MockExecuter - ): + with mock.patch("task_runner.api_methods_config.get_executer", + return_value=MockExecuter): with mock.patch.object(handler, "_pack_output", return_value=1000): yield handler From 9db87494a2a1403bc1abbbfcbe74a102387b1a18 Mon Sep 17 00:00:00 2001 From: Bruno Grego Date: Mon, 24 Nov 2025 15:24:14 +0000 Subject: [PATCH 5/5] Fix assignment and update mock --- task-runner/task_runner/task_request_handler.py | 2 +- task-runner/tests/unit/test_task_request_handler.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/task-runner/task_runner/task_request_handler.py b/task-runner/task_runner/task_request_handler.py index 9f81d0e7..d5d7c049 100644 --- a/task-runner/task_runner/task_request_handler.py +++ b/task-runner/task_runner/task_request_handler.py @@ -644,7 +644,7 @@ def _pack_output(self, output_filename: Optional[str] = None) -> int: if output_size_bytes is None: logging.error("Failed to determine size for output directory: %s", output_dir) - output_size_bytes == 0 + output_size_bytes = 0 if output_size_bytes == 0: return output_size_bytes diff --git a/task-runner/tests/unit/test_task_request_handler.py b/task-runner/tests/unit/test_task_request_handler.py index 3f60c404..85f4afd2 100644 --- a/task-runner/tests/unit/test_task_request_handler.py +++ b/task-runner/tests/unit/test_task_request_handler.py @@ -134,9 +134,9 @@ def fixture_task_request_handler( ) with mock.patch("task_runner.api_methods_config.get_executer", - return_value=MockExecuter): - with mock.patch.object(handler, "_pack_output", return_value=1000): - yield handler + return_value=MockExecuter), mock.patch.object( + handler, "_pack_output", return_value=1000): + yield handler def _setup_mock_task(