Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert!: "feat!: prep for rootPathFormat becoming ALL UPPERS (#222)" #243

Merged
merged 1 commit into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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