feat(job_attachments)!: Add OS-specific FileSystemPermissionSettings …

…classes (#41)

BREAKING CHANGE:
- Separates the `FileSystemPermissionSettings` class into two distinct types - one for Posix, and one for Windows to better accommodate OS-specific permission handling. So, added two new classes:
  - `PosixFileSystemPermissionSettings`: it is actually identical to the previous FileSystemPermissionSettings class, with the name being the only change to explicitly indicate the OS, Posix.
  - `WindowsFileSystemPermissionSettings`: A placeholder class for Windows. Will be implemented in a follow-up PR.
- The `FileSystemPermissionSettings` is now the Union type of those two classes.
- When changing the file ownership/permissions, internally checks what kind of permission settings it is using `isinstance()` checks.

Signed-off-by: Gahyun Suh <[email protected]>
Co-authored-by: Gahyun Suh <[email protected]>

src/deadline/job_attachments/_aws/deadline.py

@@ -40,7 +40,9 @@ def get_queue(
 
     # The API returns empty fields instead of an empty dict if there are no job attachment settings. So we need to
     # double check if the s3BucketName is set.
-    if response["jobAttachmentSettings"] and response["jobAttachmentSettings"].get("s3BucketName"):
+    if response.get("jobAttachmentSettings") and response["jobAttachmentSettings"].get(
+        "s3BucketName"
+    ):
         job_attachment_settings = JobAttachmentS3Settings(
             s3BucketName=response["jobAttachmentSettings"].get("s3BucketName", ""),
             rootPrefix=response["jobAttachmentSettings"].get("rootPrefix", ""),

src/deadline/job_attachments/download.py

@@ -44,6 +44,8 @@
     FileConflictResolution,
     FileSystemPermissionSettings,
     JobAttachmentS3Settings,
+    PosixFileSystemPermissionSettings,
+    WindowsFileSystemPermissionSettings,
 )
 from ._utils import _is_relative_to, _join_s3_paths
 
@@ -692,7 +694,31 @@ def _set_fs_group(
     Sets file system group ownership and permissions for all files and directories
     in the given paths, starting from root. It is expected that all `file_paths`
     point to files, not directories.
+
+    Raises:
+        TypeError: If the `fs_permission_settings` are not specific to the underlying OS.
     """
+    if os.name == "posix":
+        if not isinstance(fs_permission_settings, PosixFileSystemPermissionSettings):
+            raise TypeError(
+                "The file system permission settings must be specific to Posix-based system."
+            )
+        _set_fs_group_for_posix(
+            file_paths=file_paths,
+            local_root=local_root,
+            fs_permission_settings=fs_permission_settings,
+        )
+    else:  # if os.name is not "posix"
+        if not isinstance(fs_permission_settings, WindowsFileSystemPermissionSettings):
+            raise TypeError("The file system permission settings must be specific to Windows.")
+        # TODO: implement _set_fs_group_for_windows()
+
+
+def _set_fs_group_for_posix(
+    file_paths: list[str],
+    local_root: str,
+    fs_permission_settings: PosixFileSystemPermissionSettings,
+) -> None:
     os_group = fs_permission_settings.os_group
     dir_mode = fs_permission_settings.dir_mode
     file_mode = fs_permission_settings.file_mode

src/deadline/job_attachments/models.py

@@ -8,7 +8,7 @@
 from dataclasses import dataclass, field
 from enum import Enum
 from pathlib import Path
-from typing import List, Optional, Set, Any
+from typing import List, Optional, Set, Any, Union
 
 from deadline.job_attachments.asset_manifests.base_manifest import BaseAssetManifest
 
@@ -266,11 +266,11 @@ class FileConflictResolution(Enum):
 
 
 @dataclass
-class FileSystemPermissionSettings:
+class PosixFileSystemPermissionSettings:
     """
-    A data class representing file system permission-related information.
-    The specified permission modes will be bitwise-OR'ed with the directory
-    or file's existing permissions.
+    A dataclass representing file system permission-related information
+    for Posix. The specified permission modes will be bitwise-OR'ed with
+    the directory or file's existing permissions.
 
     Attributes:
         os_group (str): The target operating system group for ownership.
@@ -281,3 +281,19 @@ class FileSystemPermissionSettings:
     os_group: str
     dir_mode: int
     file_mode: int
+
+
+@dataclass
+class WindowsFileSystemPermissionSettings:
+    """
+    A dataclass representing file system permission-related information
+    for Windows.
+    """
+
+    # TODO: Implement this
+
+
+# A union of different file system permission settings that are based on the underlying OS.
+FileSystemPermissionSettings = Union[
+    PosixFileSystemPermissionSettings, WindowsFileSystemPermissionSettings
+]

test/unit/deadline_job_attachments/test_download.py

@@ -54,7 +54,7 @@
 from deadline.job_attachments.models import (
     Attachments,
     FileConflictResolution,
-    FileSystemPermissionSettings,
+    PosixFileSystemPermissionSettings,
     Job,
     JobAttachmentS3Settings,
     Queue,
@@ -696,7 +696,11 @@ def test_get_job_input_output_paths_by_asset_root(
         )
 
     @mock_sts
-    def test_download_files_from_manifests_with_fs_permission_settings(
+    @pytest.mark.skipif(
+        sys.platform == "win32",
+        reason="This test is for testing file permission changes in Posix-based OS.",
+    )
+    def test_download_files_from_manifests_with_posix_fs_permission_settings(
         self,
         tmp_path: Path,
         manifest_version: ManifestVersion,
@@ -713,7 +717,7 @@ def test_download_files_from_manifests_with_fs_permission_settings(
         manifest = decode_manifest(mannifest_str)
 
         manifests_by_root = {str(tmp_path): manifest}
-        fs_permission_settings = FileSystemPermissionSettings(
+        fs_permission_settings = PosixFileSystemPermissionSettings(
             os_group="test-group",
             dir_mode=0o20,
             file_mode=0o20,
@@ -773,7 +777,7 @@ def test_download_files_from_manifests_with_fs_permission_settings(
         not (has_posix_target_user() and has_posix_disjoint_user()),
         reason="Must be running inside of the sudo_environment testing container.",
     )
-    def test_download_files_from_manifests_have_correct_group(
+    def test_download_files_from_manifests_have_correct_group_posix(
         self,
         tmp_path: Path,
         manifest_version: ManifestVersion,
@@ -803,7 +807,7 @@ def test_download_files_from_manifests_have_correct_group(
         manifest = decode_manifest(mannifest_str)
         manifests_by_root = {str(tmp_path): manifest}
 
-        fs_permission_settings = FileSystemPermissionSettings(
+        fs_permission_settings = PosixFileSystemPermissionSettings(
             os_group=posix_target_group,
             dir_mode=0o20,
             file_mode=0o20,