From 05ac6135b0628f32735996b9d2c6324d86fdd5e0 Mon Sep 17 00:00:00 2001 From: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com> Date: Mon, 25 Sep 2023 16:07:39 -0500 Subject: [PATCH] fix(job_attachment)!: Change osType to rootPathFormat (#37) BREAKING CHANGES: - Renamed the field `osType` -> `rootPathFormat` - The workarounds that allowed the use of `source_os` in the path mapping rules are removed. Signed-off-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com> Signed-off-by: Graeme McHale --- pyproject.toml | 2 +- src/deadline_worker_agent/api_models.py | 10 ++--- src/deadline_worker_agent/boto/shim.py | 6 --- .../job_entities/job_attachment_details.py | 8 ++-- .../sessions/job_entities/job_details.py | 11 +---- src/deadline_worker_agent/sessions/session.py | 34 ++++----------- test/unit/conftest.py | 6 +-- test/unit/sessions/test_job_entities.py | 5 +-- test/unit/sessions/test_session.py | 41 ------------------- 9 files changed, 24 insertions(+), 99 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9a1ca6f4..30c7cba8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ dynamic = ["version"] dependencies = [ "requests ~= 2.29", "boto3 ~= 1.26", - "deadline == 0.25.*", + "deadline == 0.28.*", "openjd-sessions == 0.2.*", # tomli became tomllib in standard library in Python 3.11 "tomli >= 1.1.0 ; python_version<'3.11'", diff --git a/src/deadline_worker_agent/api_models.py b/src/deadline_worker_agent/api_models.py index 5ff7519e..770bb244 100644 --- a/src/deadline_worker_agent/api_models.py +++ b/src/deadline_worker_agent/api_models.py @@ -206,8 +206,8 @@ class ManifestProperties(TypedDict): fileSystemLocationName: NotRequired[str] """The name of the file system location""" - osType: str - """The operating system family (linux/windows/macos) associated with the asset's rootPath""" + rootPathFormat: str + """The operating system family (posix/windows) associated with the asset's rootPath""" inputManifestPath: NotRequired[str] """A (partial) key path of an S3 object that points to a file manifest. @@ -222,11 +222,7 @@ class ManifestProperties(TypedDict): class PathMappingRule(TypedDict): - # TODO: remove sourceOs and make sourcePathFormat required - sourceOs: NotRequired[str] - """The operating system family (posix/windows) associated with the source path""" - - sourcePathFormat: NotRequired[str] + sourcePathFormat: str """The path format associated with the source path (windows vs posix)""" sourcePath: str diff --git a/src/deadline_worker_agent/boto/shim.py b/src/deadline_worker_agent/boto/shim.py index 7ba01826..81069706 100644 --- a/src/deadline_worker_agent/boto/shim.py +++ b/src/deadline_worker_agent/boto/shim.py @@ -261,12 +261,6 @@ def batch_get_job_entity( } }, "pathMappingRules": [ - { - # TODO: remove once sourceOs is removed from response - "sourceOs": "windows", - "sourcePath": "C:/windows/path", - "destinationPath": "/linux/path", - }, { "sourcePathFormat": "windows", "sourcePath": "C:/windows/path", diff --git a/src/deadline_worker_agent/sessions/job_entities/job_attachment_details.py b/src/deadline_worker_agent/sessions/job_entities/job_attachment_details.py index 7b543c77..2dd3e7b4 100644 --- a/src/deadline_worker_agent/sessions/job_entities/job_attachment_details.py +++ b/src/deadline_worker_agent/sessions/job_entities/job_attachment_details.py @@ -54,8 +54,8 @@ class JobAttachmentManifestProperties: root_path: str """The input root path to be mapped""" - os_type: str - """The operating system family (linux/windows/macos) associated with the asset's root_path""" + root_path_format: str + """The operating system family (posix/windows) associated with the asset's root_path""" file_system_location_name: str | None = None """The name of the file system location""" @@ -122,7 +122,7 @@ def from_boto( input_manifest_path=manifest_properties.get("inputManifestPath", ""), input_manifest_hash=manifest_properties.get("inputManifestHash", ""), root_path=manifest_properties["rootPath"], - os_type=manifest_properties["osType"], + root_path_format=manifest_properties["rootPathFormat"], ) for manifest_properties in job_attachments_details_data["attachments"]["manifests"] ], @@ -175,7 +175,7 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobAttachmentDetai required=False, ), Field(key="rootPath", expected_type=str, required=True), - Field(key="osType", expected_type=str, required=True), + Field(key="rootPathFormat", expected_type=str, required=True), Field( key="outputRelativeDirectories", expected_type=list, diff --git a/src/deadline_worker_agent/sessions/job_entities/job_details.py b/src/deadline_worker_agent/sessions/job_entities/job_details.py index b0992c36..0195b46b 100644 --- a/src/deadline_worker_agent/sessions/job_entities/job_details.py +++ b/src/deadline_worker_agent/sessions/job_entities/job_details.py @@ -64,12 +64,7 @@ def path_mapping_api_model_to_openjd( to the format expected by Open Job Description. effectively camelCase to snake_case""" rules: list[OPENJDPathMappingRule] = [] for api_rule in path_mapping_rules: - api_source_path_format = ( - # delete sourceOs once removed from api response - api_rule["sourcePathFormat"] - if "sourcePathFormat" in api_rule - else api_rule["sourceOs"] - ) + api_source_path_format = api_rule["sourcePathFormat"] source_path_format: PathFormat = ( PathFormat.WINDOWS if api_source_path_format.lower() == "windows" else PathFormat.POSIX ) @@ -304,9 +299,7 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData: validate_object( data=path_mapping_rule, fields=( - # TODO: remove sourceOs and make sourcePathFormat required - Field(key="sourceOs", expected_type=str, required=False), - Field(key="sourcePathFormat", expected_type=str, required=False), + Field(key="sourcePathFormat", expected_type=str, required=True), Field(key="sourcePath", expected_type=str, required=True), Field(key="destinationPath", expected_type=str, required=True), ), diff --git a/src/deadline_worker_agent/sessions/session.py b/src/deadline_worker_agent/sessions/session.py index 56742653..43e4f548 100644 --- a/src/deadline_worker_agent/sessions/session.py +++ b/src/deadline_worker_agent/sessions/session.py @@ -4,7 +4,7 @@ import os from concurrent.futures import ThreadPoolExecutor -from dataclasses import dataclass, fields +from dataclasses import dataclass from datetime import datetime, timedelta, timezone from functools import partial from logging import getLogger, LoggerAdapter @@ -55,7 +55,7 @@ PosixFileSystemPermissionSettings, JobAttachmentS3Settings, ManifestProperties, - OperatingSystemFamily, + PathFormat, ) from deadline.job_attachments.progress_tracker import ProgressReportMetadata @@ -789,7 +789,7 @@ def progress_handler(job_upload_status: ProgressReportMetadata) -> bool: ManifestProperties( rootPath=manifest_properties.root_path, fileSystemLocationName=manifest_properties.file_system_location_name, - osType=OperatingSystemFamily(manifest_properties.os_type), + rootPathFormat=PathFormat(manifest_properties.root_path_format), inputManifestPath=manifest_properties.input_manifest_path, inputManifestHash=manifest_properties.input_manifest_hash, outputRelativeDirectories=manifest_properties.output_relative_directories, @@ -840,22 +840,6 @@ def progress_handler(job_upload_status: ProgressReportMetadata) -> bool: f"Summary Statistics for file downloads:\n{download_summary_statistics}" ) - # TODO: remove path_mapping_rule workaround once deadline-cloud and openjd are both upgraded - source_path_format_key = "source_path_format" - source_os_key = "source_os" - use_source_path_format = any( - f.name == source_path_format_key for f in fields(PathMappingRule) - ) - for rule in path_mapping_rules: - if use_source_path_format: - if source_os_key in rule: - rule[source_path_format_key] = rule[source_os_key] - del rule[source_os_key] - else: - if source_path_format_key in rule: - rule[source_os_key] = rule[source_path_format_key] - del rule[source_path_format_key] - job_attachment_path_mappings = [ PathMappingRule.from_dict(rule) for rule in path_mapping_rules ] @@ -1053,12 +1037,12 @@ def _sync_asset_outputs( for manifest_properties in job_attachment_details.manifests: manifest_properties_list.append( ManifestProperties( - manifest_properties.root_path, - manifest_properties.file_system_location_name, - OperatingSystemFamily(manifest_properties.os_type), - manifest_properties.input_manifest_path, - manifest_properties.input_manifest_hash, - manifest_properties.output_relative_directories, + rootPath=manifest_properties.root_path, + fileSystemLocationName=manifest_properties.file_system_location_name, + rootPathFormat=PathFormat(manifest_properties.root_path_format), + inputManifestPath=manifest_properties.input_manifest_path, + inputManifestHash=manifest_properties.input_manifest_hash, + outputRelativeDirectories=manifest_properties.output_relative_directories, ) ) diff --git a/test/unit/conftest.py b/test/unit/conftest.py index 92f6f601..6f6eb290 100644 --- a/test/unit/conftest.py +++ b/test/unit/conftest.py @@ -13,7 +13,7 @@ AssetLoadingMethod, Attachments, ManifestProperties, - OperatingSystemFamily, + PathFormat, ) from openjd.model import SchemaVersion from openjd.sessions import ( @@ -144,7 +144,7 @@ def action_id() -> str: manifests=[ ManifestProperties( rootPath="/tmp", - osType=OperatingSystemFamily.LINUX, + rootPathFormat=PathFormat.POSIX, inputManifestPath="rootPrefix/Manifests/farm-1/queue-1/Inputs/0000/0123_input.xxh128", inputManifestHash="inputmanifesthash", outputRelativeDirectories=["test_outputs"], @@ -206,7 +206,7 @@ def job_attachment_manifest_properties( ) -> JobAttachmentManifestProperties: return JobAttachmentManifestProperties( root_path="/foo/bar", - os_type="linux", + root_path_format="posix", file_system_location_name="", input_manifest_path=f"{queue_job_attachment_settings.root_prefix}/Manifests/{farm_id}/{queue_id}/Inputs/0000/0123_input.xxh128", input_manifest_hash="inputmanifesthash", diff --git a/test/unit/sessions/test_job_entities.py b/test/unit/sessions/test_job_entities.py index 04b37297..c07fefa5 100644 --- a/test/unit/sessions/test_job_entities.py +++ b/test/unit/sessions/test_job_entities.py @@ -115,8 +115,7 @@ class TestJobEntity: pytest.param( [ { - # TODO: swap to sourcePathFormat once sourceOs removed - "sourceOs": "windows", + "sourcePathFormat": "windows", "sourcePath": "Z:/artist/windows/path", "destinationPath": "/mnt/worker/windows/path", }, @@ -488,7 +487,7 @@ def test_cache_entities( "manifests": [ { "rootPath": "/mnt/share", - "osType": "linux", + "rootPathFormat": "posix", "outputRelativeDirectories": ["output"], } ] diff --git a/test/unit/sessions/test_session.py b/test/unit/sessions/test_session.py index e270ce6f..78904756 100644 --- a/test/unit/sessions/test_session.py +++ b/test/unit/sessions/test_session.py @@ -586,47 +586,6 @@ def test_sync_asset_inputs( else: session.sync_asset_inputs(cancel=cancel, **args) # type: ignore[arg-type] - def test_job_attachments_path_mapping_rules_compatibility( - self, - session: Session, - mock_asset_sync: MagicMock, - ): - """ - Tests that the path mapping rules received from job_attachments's sync_inputs - can use both the older 'source_os' and the newer 'source_path_format' based - on whatever fields are required in openjd's PathMappingRule - - Remove this test once both openjd and job_attachments both use source_path_format - """ - # GIVEN - mock_sync_inputs: MagicMock = mock_asset_sync.sync_inputs - path_mapping_rules = [ - { - # TODO: remove sourceOs once removed - "source_os": "windows", - "source_path": "Z:/artist/windows/path", - "destination_path": "/mnt/worker/windows/path", - }, - { - "source_path_format": "posix", - "source_path": "/artist/linux", - "destination_path": "/mnt/worker/linux", - }, - ] - mock_sync_inputs.return_value = ({}, path_mapping_rules) - cancel = Event() - - sync_asset_inputs_args = { - "job_attachment_details": JobAttachmentDetails( - manifests=[], - asset_loading_method=AssetLoadingMethod.PRELOAD, - ) - } - - # WHEN / THEN - session.sync_asset_inputs(cancel=cancel, **sync_asset_inputs_args) # type: ignore[arg-type] - # No errors on generating path mapping rules - success! - class TestSessionInnerRun: """Test cases for Session._run()"""