Skip to content

Add Azure auth sample app - review feedback fixes#4027

Closed
Copilot wants to merge 2 commits intodev/paul/azure-samplesfrom
copilot/sub-pr-3988
Closed

Add Azure auth sample app - review feedback fixes#4027
Copilot wants to merge 2 commits intodev/paul/azure-samplesfrom
copilot/sub-pr-3988

Conversation

Copy link
Contributor

Copilot AI commented Mar 10, 2026

Addresses unresolved review comments from the second review pass on the AzureAuthentication sample app.

Description

Bug fixes

  • Solution file: Added missing EndProject after the AzureAuthentication project entry — the omission made the .sln structurally invalid
  • packages/.gitkeep: Fixed grammar typo ("This presence" → "The presence")

Code hygiene

  • EntryPoint.cs: SetAction lambda was returning int, which doesn't match Action<ParseResult>. Capture exit code via closure instead:
    int exitCode = 0;
    rootCommand.SetAction(parseResult => { exitCode = app.Run(options); });
    rootCommand.Parse(args).Invoke();
    return exitCode;
  • App.cs: Replaced heavyweight AKV provider instantiation (which required a DefaultAzureCredential) with a lightweight assembly-load check:
    // Before
    _ = new SqlColumnEncryptionAzureKeyVaultProvider(new DefaultAzureCredential(true));
    // After
    _ = typeof(SqlColumnEncryptionAzureKeyVaultProvider).Assembly;
  • App.cs: Removed verbose full connection string output — printing SqlConnectionStringBuilder.ToString() could leak passwords and access tokens
  • App.cs / SqlClientEventListener.cs: Defined internal const string Prefix = "[EVENT]" on SqlClientEventListener and replaced the hardcoded literal in App.cs
  • App.cs: Removed using Azure.Identity — no longer referenced after the AKV check simplification

Documentation

  • README.md: Synced the defaults table with Directory.Packages.props (6.1.x7.0.0-preview*); fixed pre-release version format in help output example (7.0.0.preview47.0.0-preview4)

Issues

Testing

Sample app only — no automated tests. Changes are structural/correctness fixes with no runtime behavior change except removal of the verbose connection string output.

Guidelines

Please review the contribution guidelines before submitting a pull request:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: paulmedynski <31868385+paulmedynski@users.noreply.github.com>
Copilot AI changed the title [WIP] Add Azure auth sample app Add Azure auth sample app - review feedback fixes Mar 10, 2026
@paulmedynski
Copy link
Contributor

This isn't what I asked for.

@github-project-automation github-project-automation bot moved this from To triage to Done in SqlClient Board Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants