feat: instance profiles for multi-instance config#64
Merged
lostmygithubaccount merged 9 commits intomainfrom Apr 8, 2026
Merged
feat: instance profiles for multi-instance config#64lostmygithubaccount merged 9 commits intomainfrom
lostmygithubaccount merged 9 commits intomainfrom
Conversation
Adds named instance configurations stored in ~/.ascend-tools/config.toml, similar to Snowflake CLI connections or AWS profiles. Users can configure multiple Ascend instances and switch between them with --instance <name> or ASCEND_INSTANCE env var. Config resolution order: CLI flags > instance config > env vars > error. Fully backward compatible — existing env var workflows continue to work unchanged when no config file is present. Key changes: - New InstanceEntry type and TOML config file parsing in ascend-tools-core - Config::with_overrides_and_instance() method with 4-level resolution - instance_config module for add/remove/list/set-default operations - New `instance` CLI subcommand (add, list, remove, set-default) - Global --instance flag and ASCEND_INSTANCE env var support - Python Client(instance="staging") and JS Client(null, null, null, "staging") - service_account_key_env field stores env var name (never the secret) - Uses toml_edit for format-preserving config file writes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Avoids confusion with the global --service-account-key flag. Clap was suggesting --service-account-key (the actual secret) when users mistyped --service-account-key-env (the env var name). Shorter flag is clearer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Clap's env attribute reads env vars and treats them as CLI-level overrides, which meant shell env vars always won over instance config. The config module already handles env vars as a proper fallback, so Clap's env handling was redundant and broke the resolution order. Also fix integration tests to pass auth via CLI flags (highest priority) so they're immune to config file state on the test machine. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix double config file read in load_instance_entry error path by using remove_entry() instead of into_iter().find() + redundant load - Add instance name validation (alphanumeric, hyphens, underscores only; reject reserved "default_instance" key) - Auto-set default_instance on first instance add when no default exists - Clear dangling default_instance reference when removing the default instance - Show effective key_env in instance add confirmation message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refactor config internals to separate pure logic from I/O: - Extract select_instance_entry() for testable instance routing - Add path-parameterized CRUD helpers (add_at, remove_at, etc.) Add 25 new tests covering all previously untested branches: - select_instance_entry: all 8 routing paths (explicit found/not-found, default found/missing, implicit fallback, available list) - parse_config_toml edge cases: only-default-key, non-table values ignored, wrong-type fields error, extra fields forward compat - CRUD filesystem tests: add creates file + auto-sets default, second add preserves default, add rejects invalid names, update existing, remove instance, remove clears dangling default, remove nonexistent errors, list empty/nonexistent, set-default, set-default nonexistent Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CLI: add instance config section, resolution order, manage instances - Python: document Client(instance="staging") and resolution order - JavaScript: document named instance constructor pattern - Rust: document Config::with_overrides_and_instance() - MCP: document config.toml and ASCEND_INSTANCE inheritance - Fix rustfmt formatting in config.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
we will probably want a corresponding UI PR to change the environment variable copy or add additional options |
- Extract load_document(), load_or_create_document(), save_document() helpers to remove 3x read+parse and 3x write boilerplate - Extract available_instances() helper for 2x duplicate listing pattern - Add DEFAULT_INSTANCE_KEY and DEFAULT_INSTANCE_NAME constants replacing 12 "default_instance" and 2 "default" magic string literals - Use print_json() in instance list for consistency with other CLI handlers - Remove unused config_dir_path() - Remove unnecessary WHAT comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Problem
Users working with multiple Ascend instances must manually swap three environment variables every time they switch contexts.
Summary
Adds named instance configurations stored in
~/.ascend-tools/config.toml, with 4-level credential resolution: CLI flags > instance config > env vars > error.flowchart LR CLI["CLI flags"] --> R[resolve] TOML["config.toml"] --> R ENV["env vars"] --> R R --> Configinstancesubcommand:add,list,remove,set-default--instance <name>flag (also readsASCEND_INSTANCEenv var)service_account_key_envstores the env var name, not the secretdefault_instanceon first add; clears dangling reference on removeClient(instance="staging"), JSnew Client(null, null, null, "staging")🤖 Generated with Claude Code