Skip to content

Commit

Permalink
Change version mismatch logic to ignore Python micro # by default (#1601
Browse files Browse the repository at this point in the history
)
  • Loading branch information
LeiGlobus authored Jul 8, 2024
1 parent 9bcdc47 commit 35430cc
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 35 deletions.
36 changes: 30 additions & 6 deletions compute_sdk/globus_compute_sdk/sdk/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import dill
import globus_compute_sdk.version as sdk_version
import packaging.version
from packaging.version import Version


def chunk_by(iterable: t.Iterable, size) -> t.Iterable[tuple]:
Expand Down Expand Up @@ -47,11 +49,14 @@ def get_env_details() -> t.Dict[str, t.Any]:
}


def check_version(task_details: dict | None) -> str | None:
def check_version(task_details: dict | None, check_py_micro: bool = True) -> str | None:
"""
This method adds an optional string to some Exceptions below to
warn of differing environments as a possible cause. It is pre-pended
:param task_details: Task details from worker environment
:param check_py_minor: Whether Python micro version mismatches should
trigger a warning, or only major/minor diffs
"""
if task_details:
worker_py = task_details.get("python_version", "UnknownPy")
Expand All @@ -60,15 +65,34 @@ def check_version(task_details: dict | None) -> str | None:
worker_epid = task_details.get("endpoint_id", "<unknown>")
worker_sdk = task_details.get("globus_compute_sdk_version", "UnknownSDK")
client_details = get_env_details()

sdk_py = client_details["python_version"]
python_mismatch = True
if worker_py != "UnknownPy":
try:
sdk_v = Version(sdk_py)
worker_v = Version(worker_py)
python_mismatch = (sdk_v.major, sdk_v.minor) != (
worker_v.major,
worker_v.minor,
)
if python_mismatch is False and check_py_micro:
python_mismatch = sdk_v.micro != worker_v.micro
except packaging.version.InvalidVersion:
# Python version invalid mostly from mocks, handle as mismatch
pass

sdk_dill = client_details["dill_version"]
if (sdk_py, sdk_dill) != (worker_py, worker_dill):
if python_mismatch or sdk_dill != worker_dill:
return (
"Warning - dependency versions are mismatched between local SDK "
f"and remote worker on endpoint {worker_epid}: "
f"local SDK uses Python {sdk_py}/Dill {sdk_dill} "
"Local SDK and remote worker execution environment differences "
f"detected for endpoint {worker_epid} -\n"
f" SDK uses Python {sdk_py}/Dill {sdk_dill} "
f"but worker used {worker_py}/{worker_dill}\n"
f"(worker SDK version: {worker_sdk}; worker OS: {worker_os})"
f" (worker SDK version: {worker_sdk}; worker OS: {worker_os})\n"
"Dill and Python version mismatches can cause issues during "
"function/argument (de)serialization. Globus Compute recommends "
"keeping them in sync."
)
return None

Expand Down
58 changes: 29 additions & 29 deletions compute_sdk/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,24 +353,32 @@ def test_missing_task_info(gcc):


@pytest.mark.parametrize(
"dill_python",
("worker_dill", "worker_py", "should_warn"),
[
[None, None],
# Impossible dill and python versions will always warn
["000.dill", None],
[None, "1.2.3"],
["000.dill", "1.2.3"],
# None means use the same dill/python version as the SDK
# Impossible dill versions should always warn
[None, None, False],
["000.dill", "1.1.8", True],
["000.dill", None, True],
[None, "1.1.8", True],
],
)
def test_version_mismatch_from_details(mocker, gcc, mock_response, dill_python):
worker_dill, worker_python = dill_python

def test_version_mismatch_from_details(
mocker,
gcc,
mock_response,
worker_dill,
worker_py,
should_warn,
):
# Returned env same as client by default
response_details = get_env_details()
if worker_dill:
response_details["dill_version"] = worker_dill
if worker_python:
response_details["python_version"] = worker_python
sdk_details = get_env_details()
sdk_py = sdk_details["python_version"]

if worker_dill is None:
worker_dill = sdk_details["dill_version"]

task_py = worker_py if worker_py else sdk_py

tid = str(uuid.uuid4())
result = "some data"
Expand All @@ -381,8 +389,8 @@ def test_version_mismatch_from_details(mocker, gcc, mock_response, dill_python):
"completion_t": "1677183605.212898",
"details": {
"os": "Linux-5.19.0-1025-aws-x86_64-with-glibc2.35",
"python_version": response_details["python_version"],
"dill_version": response_details["dill_version"],
"dill_version": worker_dill,
"python_version": task_py,
"globus_compute_sdk_version": "2.3.2",
"task_transitions": {
"execution-start": 1692742841.843334,
Expand All @@ -396,17 +404,12 @@ def test_version_mismatch_from_details(mocker, gcc, mock_response, dill_python):

assert gcc.get_result(tid) == result

should_warn = worker_dill is not None or worker_python is not None
assert mock_log.warning.called == should_warn
if worker_dill or worker_python:
if should_warn:
a, *_ = mock_log.warning.call_args
assert "Warning - dependency versions are mismatched" in a[0]
if worker_dill and worker_python:
assert f"but worker used {worker_python}/{worker_dill}" in a[0]
if worker_python:
assert f"but worker used {worker_python}" in a[0]
if worker_dill:
assert f"/{worker_dill}\n(worker SDK version" in a[0]
assert "environment differences detected" in a[0]
assert f"but worker used {task_py}/{worker_dill}" in a[0]
assert "worker SDK version: " in a[0]


@pytest.mark.parametrize("should_fail", [True, False])
Expand Down Expand Up @@ -448,10 +451,7 @@ def test_version_mismatch_warns_on_failure_and_success(
assert task_res == result

assert mock_log.warning.called
assert (
"Warning - dependency versions are mismatched"
in mock_log.warning.call_args[0][0]
)
assert "environment differences detected" in mock_log.warning.call_args[0][0]


@pytest.mark.parametrize(
Expand Down
41 changes: 41 additions & 0 deletions compute_sdk/tests/unit/test_util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import pytest
from globus_compute_sdk.sdk.utils import check_version


@pytest.mark.parametrize(
("sdk_py", "worker_py", "check_micro", "should_warn"),
(
["3.11.8", "3.10.8", False, True],
["3.11.8", "3.10.8", True, True],
["3.11.8", "3.11.7", False, False],
["3.11.8", "3.11.7", True, True],
["3.11.8", None, False, True],
["3.11.8", None, True, True],
),
)
def test_check_py_version(mocker, sdk_py, worker_py, check_micro, should_warn):
mock_details = mocker.patch("globus_compute_sdk.sdk.utils.get_env_details")
mock_details.return_value = {
"os": "some-64bit",
"dill_version": "0.3.5.1",
"python_version": sdk_py,
"globus_compute_sdk_version": "2.22.0",
}
task_details = {
"os": "Linux-5.19.0-1025-aws-x86_64-with-glibc2.35",
"python_version": worker_py,
"dill_version": "0.3.5.1",
"globus_compute_sdk_version": "2.3.2",
"task_transitions": {
"execution-start": 1692742841.843334,
"execution-end": 1692742846.123456,
},
}
if worker_py is None:
del task_details["python_version"]

result = check_version(task_details, check_py_micro=check_micro)
if should_warn:
assert result and "environment differences detected" in result
else:
assert result is None

0 comments on commit 35430cc

Please sign in to comment.