Control-plane resolver: centralize local-vs-server dispatch (phases 1+2)#88
Merged
Conversation
Commands used to hand-roll the "is this id a local project/run or a server one?" decision as a copy-pasted get_local_project / local_run fork across 14 files. Add local::resolve — ProjectRef/resolve_project and RunRef/resolve_run (which reuses local::local_run, since store membership alone is not enough: server HF runs also live in the runs table) — and route every dispatch site through it. Reject-guard sites use the is_local() guard form; dual-mode sites carry the fetched project through the Local arm, so no second store lookup. up.rs and supervise.rs keep their direct store calls deliberately: those are lookups and the cloud-mirror bridge, not ownership dispatch. Store::open_at(dir) lets the resolver's tests run against a throwaway temp-dir store without mutating the process-global ORX_DATA_DIR, which the localbox lifecycle test owns under the parallel test runner. Phase 1 of the control-plane separation plan; a later phase abstracts the per-arm bodies behind the resolver.
Convert the six dual-mode commands (exp, runs, logs, project, report, create-experiment) from local-early-return guards to a uniform total match over the resolver, with server bodies extracted into named run_server-style fns — the mechanical hoist target for the later ControlPlane trait. Adds ExperimentRef/resolve_experiment (keyed on local_experiments) and routes exp.rs's five experiment-scoped dispatch sites plus both wait branches through it. Bodies are unchanged; credentials stay inside the server arms so local mode keeps working logged-out. Verified end-to-end against an isolated data dir + config (local arms serve from the store; server arms stop at the login check without touching the api).
sox8502
changed the title
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
1 participant
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.
What
Two stacked commits implementing phases 1–2 of the control-plane separation plan.
Commit 1 — the resolver (
src/local/resolve.rs)The single place that answers "does this id belong to the local store or the server api?":
ProjectRef/resolve_project(keyed onlocal_projects)RunRef/resolve_run— wraps the existinglocal::local_runtest (a run is local iff its experiment is inlocal_experiments; store membership alone is not enough, since server HF runs also land inruns)is_local()guards;Store::open_at(dir)so tests use throwaway temp dirs instead of mutating the process-globalORX_DATA_DIRCommit 2 — total-match dispatch +
ExperimentRefExperimentRef/resolve_experiment(keyed onlocal_experiments) —exp.rs's five experiment-scoped sites (status/cmd/desc/run/cancel) and bothwaitbranches route through itexp,runs,logs,project,report,create-experiment) convert fromif local { return … }early-returns to a uniform totalmatch { Local => local_fn, Server => run_server }, with server bodies extracted byte-for-byte into named fns — the mechanical hoist target for the laterControlPlanetraitWhat deliberately did NOT change
up.rs/supervise.rskeep direct store calls (lookups + the local→cloud mirror bridge, not ownership dispatch).client::RunvsStoredRunstill coexist — domain-type unification is Phase 3 (own PR; it touches wire formats on theupAPI and CLI output).Verification
runstable resolvesServer.cargo fmt --check/clippy -D warnings/cargo test --locked(187 passed) green on both commits.ORX_DATA_DIR+XDG_CONFIG_HOME: local arms (runs/logs/exp status) serve from the store logged-out; server arms stop at the login check without touching the api; reject-guards fire the exactunsupportedmessage.