fix: Fix process_sample_data.sh code #704
Conversation
NirajC-Microsoft
requested review from
Avijit-Microsoft,
Prajwal-Microsoft,
Roopan-Microsoft,
Vinay-Microsoft and
aniaroramsft
as code owners
There was a problem hiding this comment.
Pull Request Overview
This pull request improves the post-deployment experience for the Build Your Own Copilot Accelerator by automating resource parameter retrieval and enhancing documentation. The script now automatically fetches deployment outputs instead of requiring manual parameter input, with fallback to interactive prompts when needed.
- Enhanced
process_sample_data.shscript to automatically retrieve resource information from deployment outputs - Added comprehensive post-deployment documentation with step-by-step instructions
- Updated deployment outputs to include subscription ID and deployment name tagging for better traceability
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| infra/scripts/process_sample_data.sh | Automated resource parameter retrieval from deployment outputs with interactive fallback |
| infra/main.bicep | Added deployment name tagging and changed output to subscription ID |
| docs/AVMPostDeploymentGuide.md | New comprehensive post-deployment guide |
| azure.yaml | Updated script invocation to include resource group parameter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| fi | ||
| deploymentName=$(az group show --name "$resourceGroupName" --query "tags.DeploymentName" -o tsv) | ||
| echo "Deployment Name (from tag): $deploymentName" | ||
|
|
There was a problem hiding this comment.
The script will fail if deploymentName is empty or null. Add validation to check if the deployment name was successfully retrieved before using it in the az deployment group show command.
| # Validate that deploymentName is not empty or null | |
| if [ -z "$deploymentName" ]; then | |
| echo "Error: Deployment name could not be retrieved from resource group '$resourceGroupName'." | |
| exit 1 | |
| fi |
Copilot uses AI. Check for mistakes.
| read -rp "Enter SQL Server Name: " sqlServerName | ||
| read -rp "Enter SQL Database Name: " SqlDatabaseName | ||
| read -rp "Enter Key Vault Name: " keyvaultName | ||
| read -rp "Enter Web App Managed Identity Display Name: " webAppManagedIdentityDisplayName | ||
| read -rp "Enter Web App Managed Identity Client ID: " webAppManagedIdentityClientId | ||
| read -rp "Enter SQL Managed Identity Display Name: " sqlManagedIdentityDisplayName | ||
| read -rp "Enter SQL Managed Identity Client ID: " sqlManagedIdentityClientId |
There was a problem hiding this comment.
Inconsistent indentation in the interactive input section. Lines 382, 383, 385-388 use tabs while line 384 uses spaces. All lines should use consistent indentation.
Copilot uses AI. Check for mistakes.
| aif_resource_group="" | ||
| aif_account_resource_id="" | ||
| # Add global variable for SQL Server public access | ||
|
|
There was a problem hiding this comment.
[nitpick] Unnecessary blank line added. This empty line doesn't serve a purpose and should be removed.
Copilot uses AI. Check for mistakes.
2 participants
Purpose
This pull request introduces improvements to the post-deployment experience and resource management for the Build Your Own Copilot Accelerator. The most significant changes include enhanced automation for sample data import, updated documentation for post-deployment steps, and improved output and tagging in infrastructure deployment. These updates streamline onboarding, reduce manual input, and improve traceability of deployments.
Post-deployment automation and onboarding:
process_sample_data.shscript now automatically fetches required resource names and IDs from deployment outputs using the resource group name, reducing the need for manual parameter entry. If deployment outputs are unavailable, the script prompts the user for input interactively. [1] [2]azure.yaml) now include the resource group name as an argument, ensuring the sample data script has the necessary context. [1] [2]Documentation improvements:
docs/AVMPostDeploymentGuide.mdfile with clear, step-by-step instructions for post-deployment actions, including cloning the repository, importing sample data, configuring authentication, and cleaning up resources after failed deployments.Infrastructure outputs and tagging:
infra/main.bicep) now tags resource groups with the deployment name, improving traceability.Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information