refactor(workflow): replace env-var stage handoff with workflow context#186
Conversation
46e8c4b to
2696c7f
Compare
gknapp1
left a comment
gknapp1
left a comment
There was a problem hiding this comment.
First pass through reading the code, this seems like a reasonable approach. That said, I do need to spend some more time reading through it in detail and testing functionality before approving.
Remove the workflow-step communication that previously relied on mutable MYNA_* environment variables between Myna workflow stages. Changes: - add explicit `WorkflowContext` support for input file, current step, previous step, step class, and step index - update `MynaApp` to consume explicit workflow context first while preserving legacy environment-variable fallback for direct script invocation - run component configure/execute/postprocess stages in-process instead of launching Python wrapper subprocesses - preserve stage CLI argument behavior by temporarily patching `sys.argv` during in-process stage calls - update `myna run` and `myna sync` to pass workflow state directly to components instead of mutating `MYNA_*` variables - replace affected app/database direct env reads with context-backed accessors or app instance attributes - add regression tests for context propagation, stage argument forwarding, and absence of workflow env mutation Closes #125 Verification: - `uv run pytest` - 55 passed, 1 skipped, 11 deselected
Document the workflow context architecture and application guidance in the current root-docs layout. Extend the docs harness to require architecture documentation for sensitive changes and enforce PR template guidance/headings. Co-authored-by: White, Liam <whiteln@ornl.gov>
2696c7f to
e163152
Compare
gknapp1
left a comment
gknapp1
left a comment
There was a problem hiding this comment.
I modified some of the context manager logic to move the workflow context functionality to a dedicated module and made the fallback behavior to the previous functionality more clearly separated to allow for easier deprecation in version 2.0 release.
I've tested this locally and it works with ExaCA and AdditiveFOAM versions that Myna is compatible with (I had to add some compatibility functionality to the apps).
The CI failure of ExaCA is due to an issue with the containers repo that I have tried to fix with this PR: ORNL-MDF/containers#15
|
Fixed CI failure. The CI container is missing the ExaCA analysis executable for some reason, so had to disable the extra analysis in the |
- fixes error in input file with meshing settings - adds flexibility in solidification data concatenation from additivefoam to handle headers (different versions of additivefoam have different behaviors with outputting headers for decomposed ExacA files) - add version-specific logic to exaca app - adds base functionality to detect version of an app - exaca logic handles deprecated inputs from 2.1 release onwards - remove analysis executable from microstructure_region template - executable for ExaCA analysis isn't in the CI container for some reason - updating to try to get CI to pass, and analysis isn't used for Myna files - do have to keep analysis.json based on ExaCA app behavior, otehrwise throws an error
1a9e7ff to
2c5dffb
Compare
|
Cleaned up the git history, should be ready to merge now. |
2 participants
Summary
Replace workflow-stage communication through mutable
MYNA_*environment variables with explicitWorkflowContextpropagation, then document the architecture decision and add harness guardrails for future docs-sensitive changes.Related issues
Details
This refactor adds
WorkflowContextto carry the input file, current step, previous step, step class, and step index through workflow execution.myna runandmyna syncnow pass workflow state directly into component stages, and stage execution runs in-process while preserving CLI argument behavior by temporarily patchingsys.argv.Affected apps, databases, and metadata helpers now read workflow state through context-backed accessors or app instance attributes instead of directly depending on mutable environment variables. Legacy environment-variable fallback remains in place for direct script invocation.
The PR also updates architecture and developer documentation, adds an ADR for the workflow context decision, expands application documentation, and strengthens the docs harness so future PRs consider architecture documentation and complete the repository PR template.
Impact
Behavioral impact is medium because workflow stage execution and state propagation are changed across core workflow, component, app, and database paths. Risk is mitigated by preserving direct-invocation fallback behavior and adding regression coverage for context propagation, stage argument forwarding, and avoiding workflow environment mutation.
Maintainability improves by making workflow state explicit, testable, and less dependent on process-global environment state.
Testing strategy
Validated with:
uv run pytestpython3 scripts/check_docs_harness.pyuv run python scripts/check_docs_harness.pypython3 -m py_compile scripts/check_docs_harness.pygit diff --checkThe full test run reported 55 passed, 1 skipped, and 11 deselected.
Checklist
ARCHITECTURE.md, developer docs, and/or ADRs where needed, or explained why not.