diff --git a/compute_sdk/globus_compute_sdk/sdk/utils/__init__.py b/compute_sdk/globus_compute_sdk/sdk/utils/__init__.py index 27e9a2b26..bcafe86a0 100644 --- a/compute_sdk/globus_compute_sdk/sdk/utils/__init__.py +++ b/compute_sdk/globus_compute_sdk/sdk/utils/__init__.py @@ -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]: @@ -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") @@ -60,15 +65,34 @@ def check_version(task_details: dict | None) -> str | None: worker_epid = task_details.get("endpoint_id", "") 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 diff --git a/compute_sdk/tests/unit/test_client.py b/compute_sdk/tests/unit/test_client.py index a10f76447..7c00b3a38 100644 --- a/compute_sdk/tests/unit/test_client.py +++ b/compute_sdk/tests/unit/test_client.py @@ -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" @@ -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, @@ -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]) @@ -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( diff --git a/compute_sdk/tests/unit/test_util.py b/compute_sdk/tests/unit/test_util.py new file mode 100644 index 000000000..88d26b209 --- /dev/null +++ b/compute_sdk/tests/unit/test_util.py @@ -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