Skip to content

Commit

Permalink
revert: "feat!: prep for rootPathFormat becoming ALL UPPERS (#222)"
Browse files Browse the repository at this point in the history
This reverts commit d49c885.
  • Loading branch information
epmog committed Mar 25, 2024
1 parent 7c51151 commit 0581cbe
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 161 deletions.
50 changes: 2 additions & 48 deletions src/deadline/client/api/_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
from configparser import ConfigParser
from contextlib import contextmanager
from enum import Enum
from typing import Any, Optional
from typing import 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 @@ -109,9 +110,6 @@ 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 @@ -283,50 +281,6 @@ 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"].upper()
root_path_format_mapping[manifest["rootPath"]] = manifest["rootPathFormat"]

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


class PathFormat(str, Enum):
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
WINDOWS = "windows"
POSIX = "posix"

@classmethod
def get_host_path_format(cls) -> PathFormat:
Expand Down
73 changes: 0 additions & 73 deletions test/unit/deadline_client/api/test_api_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@
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 @@ -175,72 +171,3 @@ 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 @@ -148,9 +148,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 @@ -246,9 +246,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 @@ -376,9 +376,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 @@ -452,9 +452,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 @@ -814,7 +814,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 @@ -899,9 +899,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: 1 addition & 18 deletions 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,23 +18,6 @@ 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 0581cbe

Please sign in to comment.