Skip to content

Fixes lazy load server protocols#4984

Open
jasonb5 wants to merge 1 commit into
masterfrom
fix/refactor-servers-lazyload
Open

Fixes lazy load server protocols#4984
jasonb5 wants to merge 1 commit into
masterfrom
fix/refactor-servers-lazyload

Conversation

@jasonb5

@jasonb5 jasonb5 commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Description

  • Refactors CIME/Servers/__init__.py to use lazy loading via __getattr__ and @lru_cache
  • No longer runs shutil.which() at import time for svn, wget, globus-url-copy
  • Adds is_protocol_available(protocol) public API
  • Updates check_input_data.py to use new API
  • Adds 12 unit tests

Depends on: #4982

Checklist

  • My code follows the style guidelines of this project (black formatting)
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that exercise my feature/fix and existing tests continue to pass
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding additions and changes to the documentation

@github-actions

Copy link
Copy Markdown
Contributor

@jasonb5
jasonb5 force-pushed the fix/refactor-servers-lazyload branch from 834066b to 3bbc14f Compare June 18, 2026 20:17
@jasonb5
jasonb5 changed the base branch from master to feat/refactor-core-foundation June 18, 2026 20:21
Base automatically changed from feat/refactor-core-foundation to master July 8, 2026 17:38
@jasonb5
jasonb5 marked this pull request as ready for review July 16, 2026 16:53
@jasonb5
jasonb5 requested review from billsacks and jgfouca July 16, 2026 22:44
@jgfouca
jgfouca requested a review from Copilot July 17, 2026 15:19

Copilot AI left a comment

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.

Pull request overview

This PR refactors CIME.Servers to lazily detect protocol support on first access (rather than during module import), introduces a public is_protocol_available(protocol) API, updates input-data download logic to use the new API, and adds unit tests to cover the new behavior.

Changes:

  • Refactor CIME/Servers/__init__.py to lazy-load server classes and has_* flags via module __getattr__, and add is_protocol_available().
  • Update CIME/case/check_input_data.py to use is_protocol_available(protocol) instead of protocol in vars(CIME.Servers).
  • Add unit tests in CIME/tests/test_unit_servers.py for lazy loading, protocol availability, and backward-compatible attribute access.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
CIME/Servers/__init__.py Adds lazy protocol availability checks and lazy class/flag resolution via __getattr__.
CIME/case/check_input_data.py Switches protocol enablement checks to the new is_protocol_available() API.
CIME/tests/test_unit_servers.py Adds unit tests intended to validate lazy loading and backward-compat behaviors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CIME/Servers/__init__.py
Returns:
True if the protocol is available, False otherwise.
"""
protocol = protocol.lower()
Comment on lines +15 to +19
def test_import_does_not_run_which(self):
"""Importing CIME.Servers should not run shutil.which()."""
import CIME.Servers

assert CIME.Servers is not None
Comment on lines +21 to +34
def test_availability_not_checked_at_import(self):
"""Availability flags should be unchecked after import."""
import importlib
import sys

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

import CIME.Servers

assert CIME.Servers is not None

Comment on lines +3 to +6
These tests verify:
1. Lazy loading behavior (no subprocess at import time)
2. Backward compatibility with existing usage patterns
3. is_protocol_available() API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants