fix: deploy with separate locations for models and other foundry projects#7873
fix: deploy with separate locations for models and other foundry projects#7873therealjohn wants to merge 1 commit intoAzure:mainfrom
Conversation
- Use AZURE_AI_DEPLOYMENTS_LOCATION for model filtering, falling back to AZURE_LOCATION for compatibility. - Ensure both AZURE_LOCATION and AZURE_AI_DEPLOYMENTS_LOCATION are set during initialization. - Update success messages and error handling to reflect changes in variable names.
There was a problem hiding this comment.
Pull request overview
This PR updates the azure.ai.agents extension’s environment/location handling to distinguish the deployment region for AI/model resources from the resource group region, addressing failures when Foundry projects and RGs are in different Azure regions.
Changes:
- Prefer
AZURE_AI_DEPLOYMENTS_LOCATION(fallback toAZURE_LOCATION) when loadingAzureContext.Scope.Locationfor model filtering. - Update init flows to set/seed
AZURE_AI_DEPLOYMENTS_LOCATIONindependently fromAZURE_LOCATION. - Update the model selector to persist location changes to
AZURE_AI_DEPLOYMENTS_LOCATIONand reflect this in output messaging.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/cmd/init_models.go | Writes model deployment location updates to AZURE_AI_DEPLOYMENTS_LOCATION and updates the success message accordingly. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go | Loads location with new precedence, sets both env vars during prompting, and updates Foundry project selection to set AI deployment location and conditionally seed RG location. |
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go:756
- This warning message hardcodes
AZURE_LOCATION, but at this pointazureContext.Scope.Locationmay actually reflectAZURE_AI_DEPLOYMENTS_LOCATION(due to the new precedence inloadAzureContext). Consider adjusting the message to reference the actual variable(s) being validated (or use a generic “current location” wording) to avoid misleading users.
fmt.Printf("%s", output.WithWarningFormat(
"The current AZURE_LOCATION '%s' is not supported for this agent setup. Please choose a different location.\n",
azureContext.Scope.Location,
| if azureContext.Scope.Location != "" && locationAllowed(azureContext.Scope.Location, allowedLocations) { | ||
| return nil | ||
| // Location already set and valid — ensure AZURE_AI_DEPLOYMENTS_LOCATION is also set | ||
| return setEnvValue(ctx, azdClient, envName, "AZURE_AI_DEPLOYMENTS_LOCATION", azureContext.Scope.Location) |
There was a problem hiding this comment.
In the early-return path, azureContext.Scope.Location may have come from AZURE_AI_DEPLOYMENTS_LOCATION (via loadAzureContext). If AZURE_LOCATION is currently unset, this returns without seeding it, leaving the resource group location missing. Consider reading current env values and setting AZURE_LOCATION only when it is empty, while also ensuring AZURE_AI_DEPLOYMENTS_LOCATION is set when empty (without overwriting an existing, user-chosen RG location).
| // Set AI deployments location from the selected project. | ||
| // The project's location determines where AI models and resources are deployed. | ||
| previousLocation := azureContext.Scope.Location | ||
| azureContext.Scope.Location = selectedProject.Location | ||
| if err := setEnvValue(ctx, azdClient, envName, "AZURE_LOCATION", selectedProject.Location); err != nil { | ||
| return nil, fmt.Errorf("failed to set AZURE_LOCATION: %w", err) | ||
| if err := setEnvValue( | ||
| ctx, azdClient, envName, "AZURE_AI_DEPLOYMENTS_LOCATION", selectedProject.Location, | ||
| ); err != nil { | ||
| return nil, fmt.Errorf("failed to set AZURE_AI_DEPLOYMENTS_LOCATION: %w", err) | ||
| } | ||
|
|
||
| // Seed AZURE_LOCATION (resource group location) if not already set. | ||
| // Default to co-locating with the project; user can override before deploying. | ||
| if previousLocation == "" { | ||
| if err := setEnvValue(ctx, azdClient, envName, "AZURE_LOCATION", selectedProject.Location); err != nil { | ||
| return nil, fmt.Errorf("failed to set AZURE_LOCATION: %w", err) | ||
| } |
There was a problem hiding this comment.
previousLocation := azureContext.Scope.Location is tracking the AI deployments location (per loadAzureContext), not whether AZURE_LOCATION (resource group location) is set. If an environment has AZURE_AI_DEPLOYMENTS_LOCATION set but AZURE_LOCATION unset, this block will skip seeding AZURE_LOCATION, leaving it missing. Consider checking the actual AZURE_LOCATION env value (e.g., via Environment().GetValue/GetValues) before deciding whether to set it.
| // Use AZURE_AI_DEPLOYMENTS_LOCATION for Scope.Location (used for model filtering). | ||
| // Fall back to AZURE_LOCATION for backward compatibility with older environments. | ||
| location := envValueMap["AZURE_AI_DEPLOYMENTS_LOCATION"] | ||
| if location == "" { | ||
| location = envValueMap["AZURE_LOCATION"] | ||
| } | ||
|
|
||
| return &azdext.AzureContext{ | ||
| Scope: &azdext.AzureScope{ | ||
| TenantId: envValueMap["AZURE_TENANT_ID"], | ||
| SubscriptionId: envValueMap["AZURE_SUBSCRIPTION_ID"], | ||
| Location: envValueMap["AZURE_LOCATION"], | ||
| Location: location, |
There was a problem hiding this comment.
loadAzureContext now prefers AZURE_AI_DEPLOYMENTS_LOCATION over AZURE_LOCATION. This is behaviorally significant for model filtering and project selection; there don’t appear to be unit tests covering the precedence/fallback behavior. Consider adding tests for (1) AI var set, (2) only AZURE_LOCATION set (back-compat), and (3) neither set.
| @@ -98,7 +98,9 @@ func (a *modelSelector) updateEnvLocation(ctx context.Context, selectedLocation | |||
| } | |||
| a.azureContext.Scope.Location = selectedLocation | |||
|
|
|||
| fmt.Println(output.WithSuccessFormat("Updated AZURE_LOCATION to '%s' in your azd environment.", selectedLocation)) | |||
| fmt.Println(output.WithSuccessFormat( | |||
| "Updated AZURE_AI_DEPLOYMENTS_LOCATION to '%s' in your azd environment.", selectedLocation, | |||
| )) | |||
There was a problem hiding this comment.
updateEnvLocation now updates AZURE_AI_DEPLOYMENTS_LOCATION instead of AZURE_LOCATION, and also mutates a.azureContext.Scope.Location. There are unit tests in this package but none covering this env update path; consider adding a test to verify the correct key is written and that the in-memory AzureContext is updated accordingly.
jongio
left a comment
There was a problem hiding this comment.
Sound approach for #7670 - splitting AI deployment location (AZURE_AI_DEPLOYMENTS_LOCATION) from resource group location (AZURE_LOCATION) is the right fix.
Two items beyond what the automated review already covers:
Warning message (line 755): The warning in ensureLocation says "The current AZURE_LOCATION '...' is not supported" but after this change, Scope.Location may hold the value from AZURE_AI_DEPLOYMENTS_LOCATION. A user who set only the AI var to an unsupported region would see a misleading reference to AZURE_LOCATION. Consider "The current location '%s' is not supported..." or similar generic wording.
init_from_code.go:549: The guard if a.azureContext.Scope.Location == "" protects a call to ensureLocation. Since loadAzureContext now prefers AZURE_AI_DEPLOYMENTS_LOCATION, this guard can be false (location appears set) when AZURE_LOCATION is actually unset. Same root cause as the ensureLocation early-return gap, different call site.
This pull request updates how Azure location environment variables are handled for AI/model resources and resource groups, improving clarity and flexibility in regional configuration. The main focus is on distinguishing between the deployment location for AI resources and the general resource group location, ensuring both are set appropriately and allowing for independent changes when needed.
Environment variable handling improvements:
loadAzureContextnow prefersAZURE_AI_DEPLOYMENTS_LOCATIONfor AI/model resource deployments, falling back toAZURE_LOCATIONfor backward compatibility. (cli/azd/extensions/azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go)ensureLocationfunction now sets bothAZURE_LOCATION(for resource groups) andAZURE_AI_DEPLOYMENTS_LOCATION(for AI/model resources) to the same value by default, but allowsAZURE_AI_DEPLOYMENTS_LOCATIONto be changed independently later. (cli/azd/extensions/azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go) [1] [2] [3]Project selection and environment updates:
AZURE_AI_DEPLOYMENTS_LOCATION, andAZURE_LOCATIONis seeded only if not already set, allowing the user to override it before deployment. (cli/azd/extensions/azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go)AZURE_AI_DEPLOYMENTS_LOCATIONinstead ofAZURE_LOCATION, and the success message reflects this change. (cli/azd/extensions/azure.ai.agents/internal/cmd/init_models.go) [1] [2]Matching bicep updates: Azure-Samples/azd-ai-starter-basic#58
Fixes #7670