diff --git a/src/deadline/client/cli/_groups/job_group.py b/src/deadline/client/cli/_groups/job_group.py index 3cb918d4..f74da988 100644 --- a/src/deadline/client/cli/_groups/job_group.py +++ b/src/deadline/client/cli/_groups/job_group.py @@ -23,6 +23,7 @@ from deadline.job_attachments.models import ( FileConflictResolution, JobAttachmentS3Settings, + PathFormat, ) from deadline.job_attachments.progress_tracker import ( DownloadSummaryStatistics, @@ -265,6 +266,14 @@ def _download_job_output( queue_display_name=queue["displayName"], ) + # Get a dictionary mapping rootPath to rootPathFormat (OS) from job's manifests + root_path_format_mapping: dict[str, str] = {} + job_attachments = job.get("attachments", None) + if job_attachments: + job_attachments_manifests = job_attachments["manifests"] + for manifest in job_attachments_manifests: + root_path_format_mapping[manifest["rootPath"]] = manifest["rootPathFormat"] + job_output_downloader = OutputDownloader( s3_settings=JobAttachmentS3Settings(**queue["jobAttachmentSettings"]), farm_id=farm_id, @@ -281,8 +290,12 @@ def _download_job_output( # select alternative root paths to download to, (regardless of the auto-accept.) asset_roots = list(output_paths_by_root.keys()) for asset_root in asset_roots: - if _is_current_os_windows() != _is_path_in_windows_format(asset_root): - click.echo(_get_mismatch_os_root_warning(asset_root, is_json_format)) + root_path_format = root_path_format_mapping.get(asset_root, "") + if root_path_format == "": + # There must be a corresponding root path format for each root path, by design. + raise DeadlineOperationError(f"No root path format found for {asset_root}.") + if PathFormat.get_host_path_format_string() != root_path_format: + click.echo(_get_mismatch_os_root_warning(asset_root, root_path_format, is_json_format)) if not is_json_format: new_root = click.prompt( @@ -443,14 +456,15 @@ def _get_start_message( return f"Downloading output from Job {job_name!r} Step {step_name!r} Task {task_parameters_summary}" -def _get_mismatch_os_root_warning(root: str, is_json_format: bool) -> str: +def _get_mismatch_os_root_warning(root: str, root_path_format: str, is_json_format: bool) -> str: if is_json_format: return _get_json_line(JSON_MSG_TYPE_PATH, [root]) else: + path_format_capitalized_first_letter = root_path_format[0].upper() + root_path_format[1:] return ( "This root path format does not match the operating system you're using. " "Where would you like to save the files?\n" - f"The location was {root}, on {'Windows' if _is_path_in_windows_format(root) else 'POSIX OS'}." + f"The location was {root}, on {path_format_capitalized_first_letter}." ) @@ -534,35 +548,11 @@ def _assert_valid_path(path: str) -> None: """ Validates that the path exists and has the format of the OS currently running. """ - if not Path(path).is_dir(): + path_obj = Path(path) + if not path_obj.is_dir(): raise ValueError(f"Path {path} does not exist.") - if _is_current_os_windows() != _is_path_in_windows_format(path): - raise ValueError(f"Path {path} is not in the format of the operating system you're using.") - - -def _is_current_os_windows() -> bool: - """ - Checks whether the current OS is Windows. - """ - return sys.platform.startswith("win") - - -def _is_path_in_windows_format(path_str: str) -> bool: - """ - Checks the format of a path and determines whether it's in POSIX or Windows format. - Returns True if the path is in Windows format, False if it's in POSIX format. - - Note: - This function assumes that path_str is an absolute path. - A path is considered to be in POSIX format if it starts with "/". - For Windows format, it starts with a drive letter followed by ":" (e.g., C:). - """ - if path_str.startswith("/"): - return False - elif path_str[0:1].isalpha() and path_str[1:2] == ":": - return True - else: - raise ValueError(f"Path {path_str} is not an absolute path.") + if not path_obj.is_absolute(): + raise ValueError(f"Path {path} is not an absolute path.") @cli_job.command(name="download-output") diff --git a/src/deadline/job_attachments/models.py b/src/deadline/job_attachments/models.py index b98695e1..5ddd0249 100644 --- a/src/deadline/job_attachments/models.py +++ b/src/deadline/job_attachments/models.py @@ -88,9 +88,9 @@ class PathFormat(str, Enum): def get_host_path_format(cls) -> PathFormat: """Get the current path format.""" if sys.platform.startswith("win"): - return PathFormat.WINDOWS + return cls.WINDOWS if sys.platform.startswith("darwin") or sys.platform.startswith("linux"): - return PathFormat.POSIX + return cls.POSIX else: raise NotImplementedError(f"Operating system {sys.platform} is not supported.") diff --git a/test/unit/deadline_client/cli/test_cli_job.py b/test/unit/deadline_client/cli/test_cli_job.py index 6d4bb8a5..5c74d585 100644 --- a/test/unit/deadline_client/cli/test_cli_job.py +++ b/test/unit/deadline_client/cli/test_cli_job.py @@ -6,6 +6,7 @@ import datetime import json import os +import pytest from pathlib import Path import sys from unittest.mock import ANY, MagicMock, patch @@ -338,8 +339,8 @@ def test_cli_job_download_output_stdout_with_only_required_input( fresh_deadline_config, tmp_path: Path ): """ - Tests whether the ouptut messages printed to stdout match expected messages - when download-output command is executed. + Tests whether the output messages printed to stdout match expected messages + when `download-output` command is executed. """ with patch.object(api._session, "get_deadline_endpoint_url") as session_endpoint: session_endpoint.return_value = "some-endpoint-url" @@ -374,21 +375,26 @@ def test_cli_job_download_output_stdout_with_only_required_input( }, ] - mock_host_path_format_name = PathFormat.get_host_path_format_string() + mock_host_path_format = PathFormat.get_host_path_format() + boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] boto3_client_mock().get_job.return_value = { "name": "Mock Job", "attachments": { "manifests": [ { - "rootPath": "/root/path", - "rootPathFormat": PathFormat(mock_host_path_format_name), + "rootPath": f"{mock_root_path}", + "rootPathFormat": mock_host_path_format, + "outputRelativeDirectories": ["."], + }, + { + "rootPath": f"{mock_root_path}2", + "rootPathFormat": mock_host_path_format, "outputRelativeDirectories": ["."], - } + }, ], }, } - boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] runner = CliRunner() result = runner.invoke( @@ -425,13 +431,194 @@ def test_cli_job_download_output_stdout_with_only_required_input( assert result.exit_code == 0 -def test_cli_job_dowuload_output_stdout_with_json_format( +def test_cli_job_download_output_stdout_with_mismatching_path_format( + fresh_deadline_config, tmp_path: Path +): + """ + Tests that the `download-output` command handles cross-platform situations, + where the output files of the job submitted on Windows need to be downloaded + on non-Windows and vice versa, by verifying that the output messages printed + to stdout match expected messages. + """ + with patch.object(api._session, "get_deadline_endpoint_url") as session_endpoint: + session_endpoint.return_value = "some-endpoint-url" + config.set_setting("defaults.farm_id", MOCK_FARM_ID) + config.set_setting("defaults.queue_id", MOCK_QUEUE_ID) + + with patch.object(api, "get_boto3_client") as boto3_client_mock, patch.object( + job_group, "OutputDownloader" + ) as MockOutputDownloader, patch.object( + job_group, "_get_conflicting_filenames", return_value=[] + ), patch.object( + job_group, "round", return_value=0 + ), patch.object( + api, "get_queue_user_boto3_session" + ): + mock_download = MagicMock() + MockOutputDownloader.return_value.download_job_output = mock_download + + mock_root_path = "C:\\Users\\username" if sys.platform != "win32" else "/root/path" + mock_files_list = ["outputs/file1.txt", "outputs/file2.txt", "outputs/file3.txt"] + MockOutputDownloader.return_value.get_output_paths_by_root.side_effect = [ + { + f"{mock_root_path}": mock_files_list, + }, + { + str(tmp_path): mock_files_list, + }, + { + str(tmp_path): mock_files_list, + }, + ] + + # Get the opposite path format of the current operating system + current_format = PathFormat.get_host_path_format() + other_format = ( + PathFormat.WINDOWS if current_format == PathFormat.POSIX else PathFormat.POSIX + ) + + boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] + boto3_client_mock().get_job.return_value = { + "name": "Mock Job", + "attachments": { + "manifests": [ + { + "rootPath": f"{mock_root_path}", + "rootPathFormat": other_format, + "outputRelativeDirectories": ["."], + }, + ], + }, + } + + runner = CliRunner() + result = runner.invoke( + main, + ["job", "download-output", "--job-id", MOCK_JOB_ID, "--output", "verbose"], + input=f"{str(tmp_path)}\ny\n", + ) + + MockOutputDownloader.assert_called_once_with( + s3_settings=JobAttachmentS3Settings(**MOCK_GET_QUEUE_RESPONSE["jobAttachmentSettings"]), # type: ignore + farm_id=MOCK_FARM_ID, + queue_id=MOCK_QUEUE_ID, + job_id=MOCK_JOB_ID, + step_id=None, + task_id=None, + session=ANY, + ) + + assert ( + f"""Downloading output from Job 'Mock Job' +This root path format does not match the operating system you're using. Where would you like to save the files? +The location was {mock_root_path}, on {other_format[0].upper() + other_format[1:]}. +> Please enter a new root path: {str(tmp_path)} +Outputs will be downloaded to the following root paths: +[0] {str(tmp_path)} +> Please enter a number of root path to edit, y to proceed, or n to cancel the download: (0, y, n) [y]: y +""" + in result.output + ) + assert "Download Summary:" in result.output + assert result.exit_code == 0 + + +@pytest.mark.skipif( + sys.platform != "win32", + reason="This test is for testing Windows downloading job outputs located at a UNC root path.", +) +def test_cli_job_download_output_handles_unc_path_on_windows(fresh_deadline_config, tmp_path: Path): + """ + This tests only runs on Windows OS. + Executes the `download-output` command on a job that has a root path of UNC format, + and verifies that the output messages printed to stdout match expected messages. + """ + with patch.object(api._session, "get_deadline_endpoint_url") as session_endpoint: + session_endpoint.return_value = "some-endpoint-url" + config.set_setting("defaults.farm_id", MOCK_FARM_ID) + config.set_setting("defaults.queue_id", MOCK_QUEUE_ID) + + with patch.object(api, "get_boto3_client") as boto3_client_mock, patch.object( + job_group, "OutputDownloader" + ) as MockOutputDownloader, patch.object( + job_group, "_get_conflicting_filenames", return_value=[] + ), patch.object( + job_group, "round", return_value=0 + ), patch.object( + api, "get_queue_user_boto3_session" + ): + mock_download = MagicMock() + MockOutputDownloader.return_value.download_job_output = mock_download + + # UNC format (which refers to the same location as 'C:\Users\username') + mock_root_path = "\\\\127.0.0.1\\c$\\Users\\username" + mock_files_list = ["outputs/file1.txt", "outputs/file2.txt", "outputs/file3.txt"] + MockOutputDownloader.return_value.get_output_paths_by_root.side_effect = [ + { + f"{mock_root_path}": mock_files_list, + }, + { + f"{mock_root_path}": mock_files_list, + }, + { + str(tmp_path): mock_files_list, + }, + ] + + boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] + boto3_client_mock().get_job.return_value = { + "name": "Mock Job", + "attachments": { + "manifests": [ + { + "rootPath": f"{mock_root_path}", + "rootPathFormat": PathFormat.WINDOWS, + "outputRelativeDirectories": ["."], + }, + ], + }, + } + + runner = CliRunner() + result = runner.invoke( + main, + ["job", "download-output", "--job-id", MOCK_JOB_ID, "--output", "verbose"], + input=f"0\n{str(tmp_path)}\ny\n", + ) + + MockOutputDownloader.assert_called_once_with( + s3_settings=JobAttachmentS3Settings(**MOCK_GET_QUEUE_RESPONSE["jobAttachmentSettings"]), # type: ignore + farm_id=MOCK_FARM_ID, + queue_id=MOCK_QUEUE_ID, + job_id=MOCK_JOB_ID, + step_id=None, + task_id=None, + session=ANY, + ) + + assert ( + f"""Downloading output from Job 'Mock Job' +Outputs will be downloaded to the following root paths: +[0] {mock_root_path} +> Please enter a number of root path to edit, y to proceed, or n to cancel the download: (0, y, n) [y]: 0 +> Please enter a new path for the root directory [{mock_root_path}]: {str(tmp_path)} +Outputs will be downloaded to the following root paths: +[0] {str(tmp_path)} +> Please enter a number of root path to edit, y to proceed, or n to cancel the download: (0, y, n) [y]: y +""" + in result.output + ) + assert "Download Summary:" in result.output + assert result.exit_code == 0 + + +def test_cli_job_download_output_stdout_with_json_format( fresh_deadline_config, tmp_path: Path, ): """ - Tests whether the ouptut messages printed to stdout match expected messages - when download-output command is executed with `--output json` option. + Tests whether the output messages printed to stdout match expected messages + when `download-output` command is executed with `--output json` option. """ with patch.object(api._session, "get_deadline_endpoint_url") as session_endpoint: session_endpoint.return_value = "fake-endpoint-url" @@ -466,21 +653,26 @@ def test_cli_job_dowuload_output_stdout_with_json_format( }, ] - mock_host_path_format_name = PathFormat.get_host_path_format_string() + mock_host_path_format = PathFormat.get_host_path_format() + boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] boto3_client_mock().get_job.return_value = { "name": "Mock Job", "attachments": { "manifests": [ { - "rootPath": "/root/path", - "rootPathFormat": PathFormat(mock_host_path_format_name), + "rootPath": f"{mock_root_path}", + "rootPathFormat": mock_host_path_format, "outputRelativeDirectories": ["."], - } + }, + { + "rootPath": f"{mock_root_path}2", + "rootPathFormat": mock_host_path_format, + "outputRelativeDirectories": ["."], + }, ], }, } - boto3_client_mock().get_queue.side_effect = [MOCK_GET_QUEUE_RESPONSE] runner = CliRunner() result = runner.invoke(