ESMCI / ESMCI/cime

Refactor Slice 1: Foundation (typed exceptions, bootstrap, kill star imports)

Open
#4,996 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
174
Forks
225
Avg merge
1d 16h
Merged PRs (30d)
14

Description

Part of #4950. Slice 1 of 5. Estimated 3 weeks.

## Goal

Establish foundation for the refactor: typed exception hierarchy, centralized
`sys.path` bootstrap, and elimination of the worst cross-cutting code smells.
**No behavior changes** — this slice is groundwork.

## Tasks

### Exception hierarchy
- [ ] Create `CIME/core/exceptions.py` with typed hierarchy extending the
existing `CIMEError`.
- [ ] Move `CIMEError` from `utils.py:155` to `core/exceptions.py`; re-export
from `utils.py`.
- [ ] Add typed subclasses (`ConfigurationError`, `BuildError`, `BatchError`,
`CaseError`, etc.) — no callers yet, just establishing the hierarchy.

### Bootstrap module
- [ ] Create `CIME/core/config/bootstrap.py` consolidating the `sys.path.insert`
calls scattered across 11+ files.
- [ ] Replace duplicate logic in `standard_script_setup.py` and
`standard_module_setup.py`.
- [ ] Migrate call-sites incrementally to use the bootstrap module.

### Move from `utils.py` → `core/`
- [ ] `CIMEError`, `expect`, `deprecate_action`, `fixup_sys_path`,
`import_from_file`, `import_and_run_sub_or_cmd`, `run_sub_or_cmd` →
`core/exceptions.py` and `core/config/bootstrap.py` as appropriate.
- [ ] Logging infrastructure (`IndentFormatter`, `set_logger_indent`,
`configure_logging`, `setup_standard_logging_options`) →
`CIME/core/logging.py`.

### Kill star imports
- [ ] Eliminate `from CIME.XML.standard_module_setup import *` (~60 files).
Currently injects `os`, `sys`, `re`, `expect`, `run_cmd` etc. into every
namespace. **Highest-impact code-quality fix in the entire refactor.**
- [ ] Eliminate `from CIME.test_status import *` (~10 files, ~30 constants).

### Kill global mutable state
- [ ] Remove `GLOBAL = {}` (`utils.py:21`). Currently used to pass `SRCROOT`
between `case.py`, `create_test.py`, and `utils.get_src_root()` via
implicit global mutation. Replace with explicit parameter passing.

### Kill import-time side effects
- [ ] `Servers/__init__.py` runs `shutil.which()` at import time for
`globus-url-copy`, `svn`, `wget`. Make discovery lazy.

### Standardize error handling
- [ ] Audit `sys.exit()` calls (~6 sites in library code) and bare
`raise RuntimeError` (~30 sites). Migrate to `CIMEError` / `expect()`
where appropriate. `sys.exit()` only in CLI entrypoints.

### Fix test infrastructure (resolves #4936)
- [ ] Split `conftest.py` so `test_unit_*.py` runs without host model config
or machine XML. Keep machine setup for system tests only.

### Misc
- [ ] Add `__init__.py` to `CIME/Tools/xmlconvertors/` and `CIME/ParamGen/`
(currently use try/except import fallbacks).
- [ ] Start breaking circular imports between `case.py:84-102` and 10 sibling
modules — moves to `core/` should reduce these.

## What we're NOT doing in Slice 1

- No protocol classes wrapping stdlib (`os.path`, `subprocess`, `time`,
`os.environ`).
- No factory functions or service locators.
- No new abstraction layers.

## Definition of done

- [ ] All existing CIME tests pass.
- [ ] Zero star imports from `standard_module_setup` and `test_status`.
- [ ] `GLOBAL = {}` eliminated; SRCROOT passed explicitly.
- [ ] `sys.path` manipulation consolidated in `bootstrap.py`.
- [ ] No bare `sys.exit()` in library code.
- [ ] `Servers/__init__.py` doesn't run executables at import time.
- [ ] Unit tests (`test_unit_*.py`) run without host model / machine config.
- [ ] Exception hierarchy in place for later slices.
- [ ] Re-exports from `utils.py` keep external imports working.

## Related

- Resolves #4936 (testing instructions / conftest)
- Foundation for #4997, #4998, #4999, #5000

Contributor guide

Open the contributing guide

Research direction

Start by reading the listed locations in CIME/utils.py, CIME/core/, standard_script_setup.py, standard_module_setup.py, Servers/__init__.py, and conftest.py, then run the relevant test_unit_*.py tests. Done means the definition-of-done checks pass: existing tests succeed, imports and sys.path setup are consolidated, global state and library sys.exit() calls are removed, and unit tests need no host configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
build-system, testing, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.