Skip to content

Commit

Permalink
feat(job_attachments)!: Add OS-specific FileSystemPermissionSettings …
Browse files Browse the repository at this point in the history
…classes

BREAKING CHANGE:
- Separates the `FileSystemPermissionSettings` class into distinct implementations for Posix and Windows to better accommodate OS-specific permission handling.
- Two new classes
  - `PosixFileSystemPermissionSettings`: it is identical to the previous FileSystemPermissionSettings class, with the name being the only change to explicitly indicate the OS.
  - `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.

Signed-off-by: Gahyun Suh <[email protected]>
  • Loading branch information
Gahyun Suh committed Sep 14, 2023
1 parent b4b6d3c commit 950e45f
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
26 changes: 26 additions & 0 deletions src/deadline/job_attachments/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
FileConflictResolution,
FileSystemPermissionSettings,
JobAttachmentS3Settings,
PosixFileSystemPermissionSettings,
WindowsFileSystemPermissionSettings,
)
from ._utils import _is_relative_to, _join_s3_paths

Expand Down Expand Up @@ -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:
ValueError: If the `fs_permission_settings` are not specific to the underlying OS.
"""
if os.name == "posix":
if not isinstance(fs_permission_settings, PosixFileSystemPermissionSettings):
raise ValueError(
"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 ValueError("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
Expand Down
26 changes: 21 additions & 5 deletions src/deadline/job_attachments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand All @@ -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
]
6 changes: 3 additions & 3 deletions test/unit/deadline_job_attachments/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
from deadline.job_attachments.models import (
Attachments,
FileConflictResolution,
FileSystemPermissionSettings,
PosixFileSystemPermissionSettings,
Job,
JobAttachmentS3Settings,
Queue,
Expand Down Expand Up @@ -713,7 +713,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,
Expand Down Expand Up @@ -803,7 +803,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,
Expand Down

0 comments on commit 950e45f

Please sign in to comment.