Repository navigation
refactor(cli): reuse config for aliases and served clusters - #1040
Closed
aaltshuler wants to merge 2 commits into
Closed
aaltshuler wants to merge 2 commits into
aaltshuler wants to merge 2 commits into
Conversation
5 tasks done
Contributor
Author
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.
What & why
Operator aliases currently load their binding from one operator configuration, then reread the file for server and credential selection. Replacing the file while
--params-fileis being read can redirect the alias to another server. Served cluster commands likewise read the file separately for endpoint and credential selection, repeating configuration warnings.Pass the alias caller's captured configuration through execution setup and load it once in
RemoteCluster. Remove the obsolete server/token helpers that loaded configuration implicitly. Keep live credential lookup, root versus graph-specific credential matching, alias parameter precedence, and literal/named error ordering. Update the user guide, architecture guide and changelog.Backing issue / RFC
Fourth CLI refactor slice discussed in #675. Depends on #1039: this branch starts at
112a3980; the new commit contains this slice. The PR targets main and therefore includes that prerequisite until it is merged and this branch is rebased.Checklist
Local verification
Clean baseline: 19 unit tests and six integration journeys passed, including the existing ignored live-apply journey run explicitly. Before production changes, the new alias regression failed because the destination changed and the named-cluster regression failed because configuration warnings appeared twice. The error-precedence regression passed on both versions.
After the change: all 29 focused tests pass (19 unit, three new integration tests, seven compatibility journeys). The Unix alias regression uses a bounded FIFO handshake to replace configuration after binding selection; it has no timing race or production hook. It verifies graph/query destination, graph-specific credentials, parameter precedence and warning counts. Cluster coverage checks named/literal setup, token precedence, unauthenticated discovery and invalid-address errors preceding malformed-config errors.
Cargo ran from the worktree with
CARGO_TARGET_DIR=/Users/andrew/code/omnigraph/target. Compatibility journeys usedCARGO_BIN_EXE_omnigraph-server=/Users/andrew/code/omnigraph/target/debug/omnigraph-serverfrom the same main baseline.Notes for reviewers
This completes captured configuration for ordinary remote-client setup. Rendering still loads its own preferences, and managed provisioning keeps its existing owners. The CLI grammar, engine and HTTP contracts are unchanged. Tests exercise the CLI process boundary; GQT retains query-semantic ownership. Full workspace tests were not run locally; CI remains pending.