-
Notifications
You must be signed in to change notification settings - Fork 14
Pr-62-diberry-fix #63
base: main
Are you sure you want to change the base?
Conversation
…riable validation from the Frontend side to Backend side
glaucia86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Pull Request belongs to the #62
…ices re-export file to avoid expose AOIA key environments
…ecated services file
Why Sensitive Data in
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the codebase for better secret management and network isolation, while updating the Azure OpenAI integration and TypeScript type safety. Key changes include:
- Integrating Azure Key Vault for secure secret management and validating the AZURE_OPENAI_API_KEY.
- Adding a virtual network module for improved resource isolation.
- Refactoring the Azure OpenAI service implementation with updated import types and utilizing the deploymentName from environment variables.
Reviewed Changes
Copilot reviewed 6 out of 14 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/src/config/env.ts | Added runtime check for AZURE_OPENAI_API_KEY to enforce secret management. |
| app/services/openai-service.server.ts | Updated client configuration to use deploymentName and adjusted import types; consideration for cleanup of commented code. |
| app/routes/generate.tsx | Updated type casts for form data and corrected the import path for the OpenAI service. |
| .github/copilot-instructions.md | Added an instruction for Azure best practices to guide code generation. |
Files not reviewed (8)
- Dockerfile: Language not supported
- infra/abbreviations.json: Language not supported
- infra/app/microblog-app.bicep: Language not supported
- infra/main.bicep: Language not supported
- infra/shared/cognitiveservices.bicep: Language not supported
- infra/shared/vnet.bicep: Language not supported
- server/package.json: Language not supported
- server/tsconfig.json: Language not supported
Comments suppressed due to low confidence (1)
app/services/openai-service.server.ts:3
- [nitpick] Consider removing the commented-out import block if it is no longer needed to improve code clarity and maintainability.
/*import type { ChatCompletionSystemMessageParam, ChatCompletionUserMessageParam, ChatCompletionAssistantMessageParam } from "openai/resources/index.mjs";*/
|
|
||
| constructor() { | ||
| this.validateEnvVariables(); | ||
| this.deploymentName = process.env.AZURE_OPENAI_DEPLOYMENT_NAME!; |
Copilot
AI
Apr 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AZURE_OPENAI_DEPLOYMENT_NAME environment variable is used with a non-null assertion without prior validation. Consider adding an explicit check (similar to AZURE_OPENAI_API_KEY) to ensure it is defined.
| this.deploymentName = process.env.AZURE_OPENAI_DEPLOYMENT_NAME!; | |
| this.deploymentName = process.env.AZURE_OPENAI_DEPLOYMENT_NAME; |
Purpose
This Pull Request belongs to #62
This pull request introduces significant updates across multiple areas, including infrastructure configuration, application code, and environment setups. The key changes include transitioning to Key Vault for managing secrets, adding support for virtual networks, and refining the integration with Azure OpenAI services. Below is a categorized summary of the most important changes:
Infrastructure Enhancements
azure-openai-api-key,azure-openai-endpoint,azure-openai-deployment-name, andazure-openai-api-versionare now stored and referenced directly from Azure Key Vault, replacing inline secret definitions. This improves security and centralizes secret management (infra/app/microblog-app.bicep,infra/main.bicep,infra/shared/keyvault-secret.bicep). [1] [2] [3]vnet) creation, including subnet configurations. This enables better network isolation and resource organization (infra/main.bicep,infra/shared/vnet.bicep). [1] [2]Application Code Updates
formDatavalues tostringto ensure type safety in theactionfunction (app/routes/generate.tsx).AzureOpenAIServiceclass to usedeploymentNamefrom environment variables and adjusted themodelparameter to reference the deployment directly (server/src/services/openai-service.server.ts). [1] [2]Configuration and Validation
AZURE_OPENAI_API_KEYis configured, preventing misconfigurations during deployment (server/src/config/env.ts).Miscellaneous
openai-service.tstoserver/src/services/openai-service.server.tsto align with server-side responsibilities (server/src/services/openai-service.server.ts).dist/to.gitignoreto exclude build artifacts from version control (server/.gitignore).Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
What to Check
Verify that the following are valid
Other Information