Refactor: decouple stage submit config from settingsState#206
Open
pcolt wants to merge 5 commits into
Open
Conversation
- refactor: widen CoralJobConfig/ExecutableJobConfig with coralBinaryPath/coralPluginPath/executablePath/parametersFileName as required fields, so config is the complete argument bag for a stage submit - refactor: submitCoralStageRemote/submitExecutableStageRemote and buildBatchScript no longer read settingsState.remote directly — every run-mechanics input flows in via config - refactor: JobConfigModal reads settingsState once at the UI boundary and passes paths explicitly, for both remote and local dispatch - refactor: move executablePath/parametersFileName off ExecutablePipelineStage onto its config; PipelineCanvas captures coralBinaryPath/coralPluginPath at coral stage creation (symmetric to the existing executable capture) - fix: give every single remote run its own run-<ts> working subdirectory, and every local executable run its own run-<jobId> subdirectory, fixing a clobber bug on rapid back-to-back runs - test: pipelineOrchestrator.test.ts mocks sshMessages's submit/poll exports via vi.mock to test the topo-sort + submit-loop driver without Electron - docs: update CHANGELOG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It closes #203
Overview
Main goal — decouple stage submission from
settingsState. The single-stage submit primitives (submitCoralStageRemote/submitExecutableStageRemote) still readsettingsState.remotedirectly for run-mechanics inputs (coralBinaryPath,coralPluginPath,executablePath) instead of taking them as explicit arguments. That's a real coupling problem for the pipeline driver in particular:runPipelineRemotehas no business knowing about the app's settings store, so any hidden settings read inside a primitive it calls leaks an implicit dependency into what should be a pure "submit this stage" call.Follow-on idea —
configbecomes the complete, serializable argument bag for a single job/stage submit. Removing the settings reads only works if every run-mechanics field a submit needs is already sitting onconfig. Today it isn't:parametersFileNamelives in two places (ExecutablePipelineStagetop level and optionallyExecutableJobConfig) with disagreeing authority, andCoralJobConfighas no equivalent ofexecutablePathat all (coral stages silently assume one shared install via settings). This PR widens the config types to close that gap, removes the settings reads from the primitives, and fixes every call site that relied on the old fallback.Bundled fix — unify per-run working directories. Unrelated to the above but bundled into this PR since it touches the same submit primitives: rapid back-to-back single runs currently clobber the shared
graph.json/job.sh(and, for local executable runs,parameters.json) because they all reuse one working directory. This PR also gives every run its own subdir.Implementation plan
coralBinaryPath/coralPluginPathtoCoralJobConfigandexecutablePath/parametersFileNametoExecutableJobConfigas required fields; updatebuildBatchScript/buildExecutableBatchScriptto read them fromconfig.settingsState.remotespread fromsubmitCoralStageRemote/submitExecutableStageRemote; all run-mechanics inputs come fromconfig. IPC calls andjobIdMapStatestay as-is — they are the execution mechanism, not settings reads.JobConfigModaland theexportAndEval*wrappers read paths fromsettingsStateonce at the UI boundary and pass them as explicit args — both remote and local dispatch paths, plus theDEFAULT_*_CONFIG()factories.executablePath/parametersFileNamefromExecutablePipelineStage; updatepipeline.svelte.ts,ExecutableStageNode, and the orchestrator to read/write them viaconfig. Also capturecoralBinaryPath/coralPluginPathat coral stage creation inPipelineCanvas(symmetric to the existing executable capture).run-<ts>subdir (same isolation pipeline stages already have), fixing thegraph.json/job.shclobber on rapid back-to-back runs. For local executable runs, wrap each inrun-<jobId>/with bare-named files (subdir not filename prefix, to preserve deal.II path-sniffing). Drop the deadworkingDirectoryrecorded for local runs (local resolves via the in-processlocalRunsregistry, notgetJobWorkingDirectory).npm run check,npm run check:electron,npm test;pipelineOrchestrator.test.tsmockssshMessages's submit/poll exports viavi.mockto test the topo-sort + submit-loop driver without Electron; manual remote + local verification via Docker Slurm.Also
Test plan
npm run checkandnpm run check:electronpass with the widened config types and the decoupled primitives.npm testpasses;pipelineOrchestrator.test.tsmockssshMessages's submit/poll exports and exercises the driver (submit order, dependency wiring, terminal-state reporting) without Electron.executablePathpassed explicitly from settings at the call site.--dependencyand distinct#SBATCHresources, identical to pre-refactor behaviour.run-<ts>dirs and each job runs its owngraph.json(no clobber); logs and node status still resolve in JobsTable for both.parametersFileNameland in distinctrun-<jobId>/subdirs with bare-named params files; logs resolve for both.git grep -n "settingsState" src/lib/orchestration/returns no matches exceptpipelineOrchestrator.ts's own use ofsettingsState.remote.workingDirectoryfor the pipeline base dir.git grep -n "settingsState" src/lib/utils/sshMessages.tsshows no reads insidesubmitCoralStageRemote/submitExecutableStageRemote.