Skip to content

Commit

Permalink
feat!: prep for rootPathFormat becoming ALL UPPERS (#222)
Browse files Browse the repository at this point in the history
** BREAKING CHANGE **
* The PathFormat enum's values went from all lowercase to all uppercase
* The source_path_root in the path mapping rules return value from sync_inputs went from all lowercase to all uppercase

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
  • Loading branch information
epmog authored Mar 21, 2024
1 parent f137c34 commit d49c885
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 17 deletions.
50 changes: 48 additions & 2 deletions src/deadline/client/api/_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
from configparser import ConfigParser
from contextlib import contextmanager
from enum import Enum
from typing import Optional
from typing import Any, Optional
import boto3 # type: ignore[import]
from botocore.client import BaseClient # type: ignore[import]
from botocore.credentials import CredentialProvider, RefreshableCredentials
from botocore.exceptions import ( # type: ignore[import]
ClientError,
ProfileNotFound,
)

from botocore.session import get_session as get_botocore_session

from ..config import get_setting
Expand Down Expand Up @@ -110,6 +109,9 @@ def get_boto3_client(service_name: str, config: Optional[ConfigParser] = None) -
config (ConfigParser, optional): If provided, the AWS Deadline Cloud config to use.
"""
session = get_boto3_session(config=config)

if service_name == "deadline":
return DeadlineClient(session.client(service_name))
return session.client(service_name)


Expand Down Expand Up @@ -281,6 +283,50 @@ def check_authentication_status(config: Optional[ConfigParser] = None) -> AwsAut
return AwsAuthenticationStatus.CONFIGURATION_ERROR


class DeadlineClient:
"""
A shim layer for boto Deadline client.
If a method doesn't exist here, it calls the real client
"""

_real_client: Any

def __init__(self, real_client: Any) -> None:
self._real_client = real_client
self.meta = self._real_client.meta

def create_job(self, *args, **kwargs) -> Any:
deadline_service_model = self._real_client.meta.service_model

# revert to service model style if old service model is used, entries
# here can then be deleted after their migration period
path_format_enums = deadline_service_model.shape_for("PathFormat").metadata.get("enum")
if path_format_enums[0] == path_format_enums[0].lower():
# old enums are 'windows', and 'posix'
# new enums are 'WINDOWS', and 'POSIX'
if "attachments" in kwargs:
for manifest in kwargs["attachments"]["manifests"]:
manifest["rootPathFormat"] = manifest["rootPathFormat"].lower()

return self._real_client.create_job(*args, **kwargs)

def __getattr__(self, __name: str) -> Any:
"""
Respond to unknown method calls by calling the underlying _real_client
If the underlying _real_client does not have a given method, an AttributeError
will be raised.
Note that __getattr__ is only called if the attribute cannot otherwise be found,
so if this class alread has the called method defined, __getattr__ will not be called.
This is in opposition to __getattribute__ which is called by default.
"""

def method(*args, **kwargs) -> Any:
return getattr(self._real_client, __name)(*args, **kwargs)

return method


class QueueUserCredentialProvider(CredentialProvider):
"""A custom botocore CredentialProvider for handling AssumeQueueRoleForUser API
credentials. If the credentials expire, the provider will automatically refresh
Expand Down
2 changes: 1 addition & 1 deletion src/deadline/client/cli/_groups/job_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def _download_job_output(
if job_attachments:
job_attachments_manifests = job_attachments["manifests"]
for manifest in job_attachments_manifests:
root_path_format_mapping[manifest["rootPath"]] = manifest["rootPathFormat"]
root_path_format_mapping[manifest["rootPath"]] = manifest["rootPathFormat"].upper()

job_output_downloader = OutputDownloader(
s3_settings=JobAttachmentS3Settings(**queue["jobAttachmentSettings"]),
Expand Down
12 changes: 10 additions & 2 deletions src/deadline/job_attachments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,16 @@ def _missing_(cls, value):


class PathFormat(str, Enum):
WINDOWS = "windows"
POSIX = "posix"
WINDOWS = "WINDOWS"
POSIX = "POSIX"

@classmethod
def _missing_(cls, value) -> Optional[PathFormat]:
value = value.upper()
for member in cls:
if member == value:
return member
return None

@classmethod
def get_host_path_format(cls) -> PathFormat:
Expand Down
73 changes: 73 additions & 0 deletions test/unit/deadline_client/api/test_api_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
tests the deadline.client.api functions relating to boto3.Client
"""

from typing import Dict, List
from unittest.mock import call, patch, MagicMock, ANY

import boto3 # type: ignore[import]
from deadline.client import api, config
from deadline.client.api._session import DeadlineClient

import pytest


def test_get_boto3_session(fresh_deadline_config):
Expand Down Expand Up @@ -171,3 +175,72 @@ def test_check_deadline_api_available_fails(fresh_deadline_config):
assert result is False
# It should have called list_farms with to check the API
session_mock().client("deadline").list_farms.assert_called_once_with(maxResults=1)


@pytest.mark.parametrize(
"model_enums, manifests_input, manifests_output",
[
pytest.param(
(["windows", "posix"]),
(
[
{"rootPathFormat": "WINDOWS"},
{"rootPathFormat": "POSIX"},
]
),
(
[
{"rootPathFormat": "windows"},
{"rootPathFormat": "posix"},
]
),
id="old service model - all lowers",
),
pytest.param(
(["WINDOWS", "POSIX"]),
(
[
{"rootPathFormat": "WINDOWS", "rootPath": ""},
{"rootPathFormat": "POSIX", "rootPath": ""},
]
),
(
[
{"rootPathFormat": "WINDOWS", "rootPath": ""},
{"rootPathFormat": "POSIX", "rootPath": ""},
]
),
id="new service model - ALL UPPERS",
),
],
)
def test_create_job_root_path_format_compat(
model_enums: List[str],
manifests_input: List[Dict[str, str]],
manifests_output: List[Dict[str, str]],
) -> None:
"""
create_job will eventually be updated so that rootPathFormat is all UPPERS.
Here we make sure that the shim is doing its job of:
1. Calling the underlying client method
2. Replacing all rootPathFormats to be the right casing
"""
# GIVEN
fake_client = MagicMock()
fake_path_format_shape = MagicMock()
fake_path_format_shape.metadata.get.return_value = model_enums
fake_client.meta.service_model.shape_for.return_value = fake_path_format_shape
deadline_client = DeadlineClient(fake_client)

def create_job_manifests(manifests):
return {"attachments": {"manifests": manifests}}

kwargs_input = create_job_manifests(manifests_input)
kwargs_output = create_job_manifests(manifests_output)

# WHEN
deadline_client.create_job(**kwargs_input)

# THEN
fake_client.create_job.assert_called_once_with(**kwargs_output)
22 changes: 11 additions & 11 deletions test/unit/deadline_job_attachments/test_asset_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ def test_sync_inputs_no_inputs_successful(

# THEN
expected_source_path_format = (
"windows"
"WINDOWS"
if default_job.attachments.manifests[0].rootPathFormat == PathFormat.WINDOWS
else "posix"
else "POSIX"
)
assert result_pathmap_rules == [
{
Expand Down Expand Up @@ -243,9 +243,9 @@ def test_sync_inputs_successful(

# THEN
expected_source_path_format = (
"windows"
"WINDOWS"
if job.attachments.manifests[0].rootPathFormat == PathFormat.WINDOWS
else "posix"
else "POSIX"
)
assert result_pathmap_rules == [
{
Expand Down Expand Up @@ -373,9 +373,9 @@ def test_sync_inputs_with_step_dependencies(

# THEN
expected_source_path_format = (
"windows"
"WINDOWS"
if default_job.attachments.manifests[0].rootPathFormat == PathFormat.WINDOWS
else "posix"
else "POSIX"
)
assert result_pathmap_rules == [
{
Expand Down Expand Up @@ -449,9 +449,9 @@ def test_sync_inputs_with_step_dependencies_same_root_vfs_on_posix(
# THEN
merge_manifests_mock.assert_called()
expected_source_path_format = (
"windows"
"WINDOWS"
if job.attachments.manifests[0].rootPathFormat == PathFormat.WINDOWS
else "posix"
else "POSIX"
)

assert result_pathmap_rules == [
Expand Down Expand Up @@ -758,7 +758,7 @@ def test_sync_inputs_with_storage_profiles_path_mapping_rules(
# THEN
assert result_pathmap_rules == [
{
"source_path_format": "posix",
"source_path_format": "POSIX",
"source_path": default_job.attachments.manifests[0].rootPath,
"destination_path": local_root,
}
Expand Down Expand Up @@ -843,9 +843,9 @@ def test_sync_inputs_successful_using_vfs_fallback(

# THEN
expected_source_path_format = (
"windows"
"WINDOWS"
if job.attachments.manifests[0].rootPathFormat == PathFormat.WINDOWS
else "posix"
else "POSIX"
)
assert result_pathmap_rules == [
{
Expand Down
19 changes: 18 additions & 1 deletion test/unit/deadline_job_attachments/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class TestModels:
@pytest.mark.parametrize(
("sys_os", "expected_output"),
[("win32", "windows"), ("darwin", "posix"), ("linux", "posix")],
[("win32", "WINDOWS"), ("darwin", "POSIX"), ("linux", "POSIX")],
)
def test_get_host_path_format_string(self, sys_os: str, expected_output: str):
"""
Expand All @@ -18,6 +18,23 @@ def test_get_host_path_format_string(self, sys_os: str, expected_output: str):
with patch("sys.platform", sys_os):
assert PathFormat.get_host_path_format_string() == expected_output

@pytest.mark.parametrize(
("input", "output"),
[
("windows", PathFormat.WINDOWS),
("WINDOWS", PathFormat.WINDOWS),
("wInDoWs", PathFormat.WINDOWS),
("posix", PathFormat.POSIX),
("POSIX", PathFormat.POSIX),
("PoSiX", PathFormat.POSIX),
],
)
def test_path_format_case(self, input: str, output: PathFormat) -> None:
"""
Tests that the correct enum types are created regardless of input string casing.
"""
assert PathFormat(input) == output

@pytest.mark.parametrize(
("input", "output"),
[
Expand Down

0 comments on commit d49c885

Please sign in to comment.