spmi collect/jitrollingbuild: Replace DefaultAzureCredential => AzureCliCredential #115067
Conversation
ghost
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the usage of DefaultAzureCredential with AzureCliCredential in several SuperPMI-related scripts to ensure authentication is performed using the Azure CLI method instead of the default chained credential.
- Updated imports and instantiation in src/coreclr/scripts/superpmi.py
- Replaced DefaultAzureCredential with AzureCliCredential in azure library functions within src/coreclr/scripts/jitutil.py
- Altered credential usage in BlobServiceClient initialization in src/coreclr/scripts/jitrollingbuild.py
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/coreclr/scripts/superpmi.py | Updated import and instantiation of Azure credential |
| src/coreclr/scripts/jitutil.py | Changed DefaultAzureCredential to AzureCliCredential in imports and globals |
| src/coreclr/scripts/jitrollingbuild.py | Replaced DefaultAzureCredential with AzureCliCredential in both import and credential instantiation |
| @@ -3701,14 +3701,14 @@ def list_superpmi_collections_container_via_azure_api(path_filter=lambda unused: | |||
| """ | |||
|
|
|||
| require_azure_storage_libraries() | |||
| from jitutil import ContainerClient, DefaultAzureCredential | |||
| from jitutil import ContainerClient, AzureCliCredential | |||
There was a problem hiding this comment.
The update correctly replaces the DefaultAzureCredential with AzureCliCredential. Ensure that all dependent code using this credential behaves consistently with the new authentication flow.
Copilot uses AI. Check for mistakes.
|
|
||
| superpmi_container_url = az_blob_storage_superpmi_container_uri | ||
|
|
||
| paths = [] | ||
| ok = True | ||
| try: | ||
| az_credential = DefaultAzureCredential() | ||
| az_credential = AzureCliCredential() |
There was a problem hiding this comment.
Instantiating AzureCliCredential here updates the authentication method. Confirm that the new credential’s behavior matches expectations in production scenarios.
Copilot uses AI. Check for mistakes.
| @@ -537,13 +537,13 @@ def require_azure_storage_libraries(need_azure_storage_blob=True, need_azure_ide | |||
| Once we've done it once, we don't do it again. | |||
|
|
|||
| For this to work for cross-module usage, after you call this function, you need to add a line like: | |||
| from jitutil import BlobClient, DefaultAzureCredential | |||
| from jitutil import BlobClient, AzureCliCredential | |||
There was a problem hiding this comment.
Replacing the credential import here ensures consistency with the new Azure CLI–based authentication, which aligns with the intended changes.
Copilot uses AI. Check for mistakes.
| naming all the types you want to use. | ||
|
|
||
| The full set of types this function loads: | ||
| BlobServiceClient, BlobClient, ContainerClient, DefaultAzureCredential | ||
| BlobServiceClient, BlobClient, ContainerClient, AzureCliCredential |
There was a problem hiding this comment.
Ensure that the global variable update for Azure credential is fully integrated with AzureCliCredential and that the downstream usage does not rely on any specific behavior of DefaultAzureCredential.
Copilot uses AI. Check for mistakes.
| @@ -457,7 +457,7 @@ def upload_blob(file, blob_name): | |||
|
|
|||
| try: | |||
| from azure.storage.blob import BlobServiceClient | |||
| from azure.identity import DefaultAzureCredential | |||
| from azure.identity import AzureCliCredential | |||
There was a problem hiding this comment.
Updating the import here maintains consistency with the credential change. Verify that any fallback logic depending on DefaultAzureCredential is no longer necessary.
Copilot uses AI. Check for mistakes.
| @@ -466,7 +466,7 @@ def upload_blob(file, blob_name): | |||
| logging.warning("See also https://learn.microsoft.com/azure/storage/blobs/storage-quickstart-blobs-python") | |||
| raise RuntimeError("Missing azure storage or identity packages.") | |||
|
|
|||
| default_credential = DefaultAzureCredential() | |||
| default_credential = AzureCliCredential() | |||
There was a problem hiding this comment.
The instantiation of AzureCliCredential replaces the previous credential and should provide the expected authentication behavior for BlobServiceClient. Confirm its integration in performance and error handling.
Copilot uses AI. Check for mistakes.
|
I've chatted with Jakob about this offline. AzurePipelinesCredential is the recommended cred here, but this will work. If this unblocks - LGTM |
|
/ba-g don't wait to unblock jitrollingbuild |
No description provided.