Skip to content
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
93 changes: 74 additions & 19 deletions CIME/Servers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,74 @@
# pylint: disable=import-error
from shutil import which

has_gftp = which("globus-url-copy")
has_svn = which("svn")
has_wget = which("wget")
has_ftp = True
try:
from ftplib import FTP
except ImportError:
has_ftp = False
if has_ftp:
from CIME.Servers.ftp import FTP
if has_svn:
from CIME.Servers.svn import SVN
if has_wget:
from CIME.Servers.wget import WGET
if has_gftp:
from CIME.Servers.gftp import GridFTP
"""
CIME Server implementations for data transfer.

Server availability is detected lazily on first access to avoid
running executables at import time.
"""

import shutil
from functools import lru_cache


@lru_cache(maxsize=None)
def is_protocol_available(protocol: str) -> bool:
"""
Check if a protocol is available.

Args:
protocol: One of 'ftp', 'svn', 'wget', 'gftp'

Returns:
True if the protocol is available, False otherwise.
"""
protocol = protocol.lower()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot The typing explicitly requires protocol to have a value, let's guard at the call site against a None value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 735760d: I added call-site guards so we no longer call CIME.Servers.is_protocol_available(...) when protocol is None (in both _download_checksum_file and _check_input_data_impl).

if protocol == "ftp":
try:
from ftplib import FTP # noqa: F401 pylint: disable=unused-import

return True
except ImportError:
return False
elif protocol == "svn":
return shutil.which("svn") is not None
elif protocol == "wget":
return shutil.which("wget") is not None
elif protocol == "gftp":
return shutil.which("globus-url-copy") is not None
return False


def __getattr__(name: str):
"""Lazy loading of server classes and has_* attributes."""
if name == "FTP":
if is_protocol_available("ftp"):
from CIME.Servers.ftp import FTP

return FTP
raise AttributeError("FTP server not available")
elif name == "SVN":
if is_protocol_available("svn"):
from CIME.Servers.svn import SVN

return SVN
raise AttributeError("SVN server not available (svn not found)")
elif name == "WGET":
if is_protocol_available("wget"):
from CIME.Servers.wget import WGET

return WGET
raise AttributeError("WGET server not available (wget not found)")
elif name == "GridFTP":
if is_protocol_available("gftp"):
from CIME.Servers.gftp import GridFTP

return GridFTP
raise AttributeError("GridFTP server not available (globus-url-copy not found)")
elif name == "has_ftp":
return is_protocol_available("ftp")
elif name == "has_svn":
return is_protocol_available("svn")
elif name == "has_wget":
return is_protocol_available("wget")
elif name == "has_gftp":
return is_protocol_available("gftp")
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
8 changes: 6 additions & 2 deletions CIME/case/check_input_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ def _download_checksum_file(rundir):
# download and merge all available chksum files.
while protocol is not None:
protocol, address, user, passwd, chksum_file, _, _ = inputdata.get_next_server()
if protocol not in vars(CIME.Servers):
if protocol is None:
continue
if not CIME.Servers.is_protocol_available(protocol):
logger.info("Client protocol {} not enabled".format(protocol))
continue
logger.info(
Expand Down Expand Up @@ -414,7 +416,9 @@ def _check_input_data_impl(
no_files_missing = True
server = None
if download:
if protocol not in vars(CIME.Servers):
if protocol is None:
return False
if not CIME.Servers.is_protocol_available(protocol):
logger.info("Client protocol {} not enabled".format(protocol))
return False
logger.info(
Expand Down
138 changes: 138 additions & 0 deletions CIME/tests/test_unit_servers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
"""Unit tests for CIME.Servers lazy loading.

These tests verify:
1. Lazy loading behavior (no shutil.which() / executable availability checks at import time)
2. Backward compatibility with existing usage patterns
3. is_protocol_available() API
Comment thread
jasonb5 marked this conversation as resolved.
"""

import pytest


class TestServersLazyLoading:
"""Tests for lazy loading of server modules."""

def test_import_does_not_run_which(self, monkeypatch):
"""Importing CIME.Servers should not run shutil.which()."""
import importlib
import shutil
import sys

def _boom(*_args, **_kwargs):
raise AssertionError("shutil.which() should not be called at import time")

monkeypatch.setattr(shutil, "which", _boom)

# Force reimport
for mod in list(sys.modules):
if mod == "CIME.Servers" or mod.startswith("CIME.Servers."):
del sys.modules[mod]

importlib.import_module("CIME.Servers")

def test_availability_checked_lazily_and_cached(self, monkeypatch):
"""Availability checks should run on first access and then be cached."""
import shutil

import CIME.Servers

calls = {"n": 0}

def _fake_which(_cmd):
calls["n"] += 1
return None

monkeypatch.setattr(shutil, "which", _fake_which)
CIME.Servers.is_protocol_available.cache_clear()

assert calls["n"] == 0 # no availability checks at import time

_ = CIME.Servers.has_svn
assert calls["n"] == 1

_ = CIME.Servers.has_svn
assert calls["n"] == 1


class TestProtocolAvailability:
"""Tests for is_protocol_available() API."""

def test_is_protocol_available_ftp(self):
"""FTP should always be available (stdlib)."""
import CIME.Servers

assert CIME.Servers.is_protocol_available("ftp") is True

def test_is_protocol_available_invalid(self):
"""Invalid protocol should return False."""
import CIME.Servers

assert CIME.Servers.is_protocol_available("invalid") is False

def test_is_protocol_available_case_insensitive(self):
"""Protocol check should be case-insensitive."""
import CIME.Servers

assert CIME.Servers.is_protocol_available("FTP") is True
assert CIME.Servers.is_protocol_available("Ftp") is True
assert CIME.Servers.is_protocol_available("ftp") is True


class TestServerClassAccess:
"""Tests for accessing server classes."""

def test_ftp_attribute_access(self):
"""Accessing CIME.Servers.FTP should work via __getattr__."""
import CIME.Servers

FTP = CIME.Servers.FTP
assert FTP is not None
assert FTP.__name__ == "FTP"

def test_ftp_has_ftp_login_method(self):
"""FTP class should have ftp_login class method (used by check_input_data)."""
import CIME.Servers

assert hasattr(CIME.Servers.FTP, "ftp_login")

def test_wget_has_wget_login_method(self):
"""WGET class should have wget_login class method if available."""
import CIME.Servers

if CIME.Servers.is_protocol_available("wget"):
assert hasattr(CIME.Servers.WGET, "wget_login")

def test_unavailable_server_raises_attribute_error(self):
"""Accessing unavailable server should raise AttributeError."""
import CIME.Servers

if not CIME.Servers.is_protocol_available("gftp"):
with pytest.raises(AttributeError):
_ = CIME.Servers.GridFTP


class TestBackwardCompatibility:
"""Tests for backward compatibility with existing code patterns."""

def test_has_ftp_attribute(self):
"""has_ftp attribute should return True (FTP always available)."""
import CIME.Servers

assert CIME.Servers.has_ftp is True

def test_has_attributes_exist(self):
"""All has_* attributes should be accessible."""
import CIME.Servers

# These should not raise, regardless of availability
_ = CIME.Servers.has_ftp
_ = CIME.Servers.has_svn
_ = CIME.Servers.has_wget
_ = CIME.Servers.has_gftp

def test_instantiate_ftp_server(self):
"""Should be able to instantiate FTP server (backward compat)."""
import CIME.Servers

FTP = CIME.Servers.FTP
assert FTP is not None
Loading