Refactor core foundation (exceptions and bootstrap)#4982
Conversation
…ails Rewrite plan.md, implementation.md, and README.md to reflect the reorganize-don't-rewrite philosophy, utils.py distribution strategy, cross-cutting issues inventory, and Case two-phase refactoring approach.
…trap module Add CIME.core.exceptions with CIMEError and typed subclasses (ConfigurationError, BuildError, SubmitError, etc.). CIMEError preserves dual SystemExit+Exception inheritance for traceback suppression. Add CIME.core.config.bootstrap with centralized sys.path management (bootstrap_cime, find_cimeroot, check_minimum_python_version). Include 40 unit tests with 98% coverage.
Replace the CIMEError class definition in utils.py with a re-export from CIME.core.exceptions for backward compatibility. Update 19 internal callers to import CIMEError directly from CIME.core.exceptions. External consumers using 'from CIME.utils import CIMEError' or 'utils.CIMEError' continue to work unchanged via the re-export.
|
You can preview documentation at https://esmci.github.io/cime/branch/feat/refactor-core-foundation/html/index.html |
Refactor planning and slice breakdowns previously kept in doc/dev/refactor/ are now tracked in GitHub issues to keep discussion, status, and cross-linking in one place. Master tracker: #4950 Slices: - #4996 Slice 1: Core foundation - #4997 Slice 2: Batch system extraction - #4998 Slice 3A: SRCROOT feature - #4999 Slice 3B: Build system extraction - #5000 Slice 4: Case decomposition Closes the now-superseded planning issue #4528.
jgfouca
left a comment
There was a problem hiding this comment.
Looks good to me. Can you add motivation for this change in the description?
billsacks
left a comment
There was a problem hiding this comment.
I like the direction you're taking things here - thank you for this work! I have reviewed the production code and did a cursory look at the unit tests. I have a few comments. I appreciate the added testing here.
billsacks
left a comment
There was a problem hiding this comment.
Thank you for the fixes! See the comment I made about the algorithm for inserting paths, but as I note there, I'm fine with either approach. So I'm approving here and will let you decide which way to go.
|
Test failures are unrelated. |
Description
CIME/core/exceptions.pywith typed exception hierarchy extendingCIMEErrorCIME/core/config/bootstrap.pyfor centralized sys.path managementCIMEErrorimports to use new locationMotivation
CIME's foundation has accumulated two recurring pain points that this PR addresses as the first slice of a broader
CIME/core/refactor:Typed exceptions. Today there is a single
CIMEErrorinCIME/utils.py, so callers can't distinguish a configuration problem from a build failure from a transient subprocess error without string-matching on messages. The new hierarchy (ConfigurationError,BuildError,SubmitError,SystemTestError,LockingError,ExternalCommandError,RetryableExternalCommandError,InputError) enables:exceptclauses in callers (e.g. retry only transient subprocess errors)ExternalCommandErrormodeled onsubprocess.CalledProcessError, carryingreturncode,cmd,output, andstderrsorun_cmdfailures preserve diagnostic contextCIME.utils.CIMEErroris re-exported from the new location so existing imports (internal or downstream) continue to work.Centralized sys.path bootstrap. CIME is not pip-installed; every entry script (
xmlchange,case.setup,create_newcase, etc.) does its ownsys.path.insertto makeimport CIME...work. This duplicated logic has drifted in subtle ways.bootstrap_cime()provides:__file__)standard_script_setup.pyExisting call sites are intentionally unchanged in this PR — the module is standalone so subsequent slices can adopt it incrementally.
Why these two pieces first. They are dependency-free building blocks that the planned
CIME/core/DI-based abstractions (filesystem, process, env, clock) will rely on, so they need to land before the higher-level work.Related
GLOBALremoval, and conftest fix remain.Checklist