Skip to content

refactor: rename get_async_azure_credential to build_async_azure_credential (address Copilot review on #915)#916

Merged
Avijit-Microsoft merged 1 commit into
devfrom
psl-rename-async-azure-credential-helper
May 15, 2026
Merged

refactor: rename get_async_azure_credential to build_async_azure_credential (address Copilot review on #915)#916
Avijit-Microsoft merged 1 commit into
devfrom
psl-rename-async-azure-credential-helper

Conversation

@Rafi-Microsoft
Copy link
Copy Markdown
Contributor

Purpose

Does this introduce a breaking change?

  • Yes
  • No

The renamed function is internal to src/api and only referenced from history_service.py. No public API, no config, no infra change.

Golden Path Validation

  • I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Bug 43424 fix from PR #915 was previously validated end-to-end on validation RGs rg-kmgenrf02 (australiaeast) and rg-kmgenrf03 (japaneast) on the CSA-CTO-Engineering-Maintenance subscription using branch images acrkmgenpsl02.azurecr.io/km-{api,app}:psl-fix. Single-chat delete and bulk-delete after idle session both succeed without the prior 500. This PR is a pure rename on top of that validated fix.

Deployment Validation

  • I have validated the deployment process successfully and all services are running as expected with this change.

Same validated deployment as PR #915 — no infrastructure or environment changes in this PR.

What to Check

Verify that the following are valid

  • src/api/helpers/azure_credential_utils.pyget_async_azure_credential is renamed to build_async_azure_credential; signature, branching on APP_ENV, and returned credential types are unchanged.
  • src/api/services/history_service.py — import on line 8 and call site in init_cosmosdb_client (line 49) updated to the new name. get_azure_credential_async (the original async helper, used in the async with block on line 73) is left untouched.
  • No call sites of the old name remain anywhere in the repo (verified via grep).
  • flake8 --config=.flake8 src/api passes locally.

Other Information

  • Originating Copilot comment on the merged PR:

    "Two near-identical helpers now exist: get_azure_credential_async (async function returning an async credential) and the new get_async_azure_credential (sync function returning the same async credential types). The names differ only by word order, which is easy to confuse at call sites — note that history_service.py now imports both. Consider renaming the new function to something less ambiguous (e.g., get_async_azure_credential_sync or build_async_azure_credential) to make the sync-vs-async distinction obvious at the call site."

  • Chose build_async_azure_credential (one of Copilot's two suggestions) because build_ is a common Python idiom for sync factory functions and reads more naturally than the _sync suffix variant.
  • Diff size: 2 files, +7/-6 lines.

…ential

Addresses Copilot review feedback on PR #915 (merged into dev). The two helpers get_azure_credential_async (async fn) and get_async_azure_credential (sync fn) differ only by word order, which is easy to confuse at call sites. Renaming the sync builder to build_async_azure_credential makes the sync-vs-async distinction explicit: build_ implies sync construction, async describes the returned credential type.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Coverage

Coverage Report •
FileStmtsMissCoverMissing
src/api/helpers
   azure_credential_utils.py15193%52
src/api/services
   history_service.py2062488%93, 224–225, 227, 264–266, 282, 288–290, 307, 323–324, 326, 342, 368–369, 371, 387, 407–408, 410, 429
TOTAL131415288% 

Tests Skipped Failures Errors Time
163 0 💤 0 ❌ 0 🔥 5.253s ⏱️

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Renames the sync helper get_async_azure_credential to build_async_azure_credential to disambiguate it from the existing async helper get_azure_credential_async, addressing a Copilot review comment from PR #915.

Changes:

  • Renamed function definition in azure_credential_utils.py and clarified its docstring.
  • Updated the import and call site in history_service.py to use the new name.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/api/helpers/azure_credential_utils.py Renames helper to build_async_azure_credential and improves docstring contrasting it with the async variant.
src/api/services/history_service.py Updates import and call site in init_cosmosdb_client to use the new name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Avijit-Microsoft Avijit-Microsoft merged commit 174a390 into dev May 15, 2026
9 checks passed
@Ragini-Microsoft Ragini-Microsoft mentioned this pull request May 15, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants