Add Connection Profiles documentation and command support#179
Add Connection Profiles documentation and command support#179KhushviB wants to merge 12 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to add first-class “connection profiles” to CosmosDB Shell so users can save and reuse named connection configurations, alongside documentation updates describing the new workflow.
Changes:
- Adds a new
profilecommand (save,list,use,delete) and a JSON-backed profile persistence layer. - Documents connection profiles in
docs/navigation.mdwith validation rules and examples. - Updates
connectto introduce a new (hidden)--disable-armoption and attempts to pass it into the core connection flow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| docs/navigation.md | Adds Connection Profiles documentation, plus new CLI argument and “machine mode/exit codes” documentation (currently inconsistent with implementation). |
| CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ProfileManager.cs | Introduces ProfileManager + ConnectionProfile persisted to ~/.cosmosdbshell/profiles.json. |
| CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ProfileCommand.cs | Adds the profile command implementation and wiring to persistence/connection behavior. |
| CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ConnectCommand.cs | Adds a hidden --disable-arm option and attempts to pass it to ShellInterpreter.ConnectAsync. |
… restrict saved profile fields; add ConnectCommand.ExecuteProfileAsync; fix ConnectAsync calls and error handling; refactor ExecuteAsync; update docs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
All review comments have been addressed and pushed in a single commit to this PR branch (feature/connection-profiles on the fork). Summary of fixes per feedback:\n\n1) ProfileCommand visibility\n- Fixed: made ProfileCommand internal to match other command classes and avoid expanding public API.\n\n2) Using directives / missing usings\n- Fixed: added System.Text.Json and Azure.Data.Cosmos.Shell.Parser using where required; reorganized usings to satisfy style rules.\n\n3) Saved profile contents / secrets\n- Fixed: restricted saved profile fields to non-secret data (Endpoint, Mode). Removed LoginHint/TenantId/ManagedIdentityClientId persistence to avoid storing transient/secret info.\n\n4) ExecuteAsync switch/await mismatch\n- Fixed: replaced expression-switch with clear if/else flow to ensure async Task returns are awaited correctly.\n\n5) Invalid ConnectAsync named parameters (disableArm/connectionMode)\n- Fixed: removed invalid named parameters and updated calls to use the existing ConnectAsync parameter ordering.\n\n6) Added ConnectCommand.ExecuteProfileAsync helper\n- Fixed: added internal helper that maps a saved profile into the existing ConnectAsync flow so 'profile use ' behaves identically to manual connect.\n\n7) Error handling / ErrorCommandState usage\n- Fixed: adjusted error returns to construct ErrorCommandState using existing constructors and wrap exceptions in CommandException where appropriate.\n\n8) ProfileManager thread-safety and file path\n- Fixed: ensured ProfileManager uses a lock and writes to the user profile folder (~/.cosmosdbshell/profiles.json) as intended.\n\n9) Docs update\n- Fixed: updated docs/navigation.md to match the implemented behavior and clarified that secrets are not persisted.\n\n10) Build / style warnings\n- Fixed: resolved compile errors and addressed style warnings where feasible. The solution builds successfully locally.\n\nCommit: cc02d12 — Resolve PR review comments and build fixes (pushed to feature/connection-profiles on the fork). Let me know if you'd like me to squash or change the commit message before merging. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- fix profile command behavior and validation paths - make profile persistence atomic and resilient to malformed JSON - align docs with actual CLI options and profile behavior - add localization keys and focused profile tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Final pass complete in commit a5bdfe5 (pushed to feature/connection-profiles). I re-reviewed against issue #104 and addressed Copilot expectations end-to-end: added missing profile localization keys, fixed profile default action to list, stored full endpoint URI, unified name validation across save/use/delete, made delete fail when profile is missing, escaped list table values, made profile persistence atomic, handled malformed JSON safely, normalized case-collision keys safely, preserved cancellation semantics in profile connect flow, removed ineffective disable-arm connect option, corrected docs to match real CLI options/behavior, and added focused ProfileCommand/ProfileManager tests. |
- remove undocumented profile set alias - escape unknown-action rendering path - clarify endpoint-only profile auth behavior in docs/code - make profile tests cross-platform by scoping HOME and USERPROFILE Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed review #4701862338 in commit ca25a41.\n\nFixed items:\n1) Removed undocumented profile set alias (only save/list/use/delete remain).\n2) Unknown profile action now renders through error formatting (escaped output path).\n3) Clarified profile auth semantics in docs: profiles persist endpoint/mode only and rely on current credential context.\n4) Updated profile tests to set/restore both USERPROFILE and HOME for cross-platform isolation.\n\ndotnet build CosmosDBShell/CosmosDBShell.csproj succeeds.\nTargeted test execution is currently blocked in this environment by Application Control policy (process launch denied for test host), so I could not run the filtered test command here. |
… write, and doc accuracy
- localize profile list headers and default mode label - make profile error rendering consistent - handle access-denied profile loads gracefully - document profile command in commands reference - clarify endpoint-only profile auth behavior in docs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
||
| public override async Task<CommandState> ExecuteAsync(ShellInterpreter shell, CommandState commandState, string commandText, CancellationToken token) | ||
| { | ||
| var action = (this.Action ?? "list").Trim().ToLowerInvariant(); |
| if (string.IsNullOrWhiteSpace(profile.Endpoint)) | ||
| { | ||
| throw new CommandException("profile", "Saved profile is missing an endpoint. Re-save the profile."); | ||
| } |
| else | ||
| { | ||
| throw new CommandException("profile", $"Invalid mode '{profile.Mode}'. Re-save the profile."); | ||
| } |
|
It's not about the feature implementation - the thing is the feature per se - connection strings are not "profiles" to begin with. And I'm not sure if this is worth having at all - and with switching environments I strongly recommend having more than 1 shell open. ps. do you have own features/issues? The current issue list is not really a todo list - more like a "discussion starting point". Not everything makes sense there. Maybe ~80% are to do items but the connection profiles - do they really make sense? |
|
@mkrueger I totally understand, I would love to wait on fully decided needs from this issue. currently I am not having my own issues/features. If you could assign me any from shell or any other repo, would love to take up those. |
Issue � Missing Connection-Profile Support (#104)
Problem
Cosmos DB Shell lacked a mechanism for users to persist, list, switch, or delete named connection profiles.
Consequences:
Fix
Implemented a
profilecommand suite:profile save <name>profile listprofile use <name>profile delete <name>Key Implementation Details
^[A-Za-z0-9_.-]{1,64}$.~/.cosmosdbshell/profiles.jsonvia a thread-safeProfileManager.endpointandmode). Secrets such as account keys are not persisted.profile useroutes through the existing connection flow using the saved endpoint/mode and current auth context (for example existing token,COSMOSDB_SHELL_ACCOUNT_KEY, or other supported ambient credentials).docs/navigation.mdanddocs/commands.md.Result
Users can now reuse named, non-secret connection metadata across sessions without retyping endpoints, while keeping credential handling outside the profile store.