-
Notifications
You must be signed in to change notification settings - Fork 158
refactor: Code clean up, EXP changes #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
* added opentelemetry log in apis * resolved pylint issues * resolved pylint issues * resolved pylint issues * resolved pylint issues * removed space * updated requirement file
* Replace Gunicorn with Uvicorn for the backend server and remove Gunicorn configuration * Update deployment instructions to use Uvicorn instead of Gunicorn
* cleanup the unused variables in all the files * fix: remove unused OPENAI_API_VERSION variable * fix: resolve unit tests issue
feat: EXP environment changes for Log Analytics workspace
feat: EXP environment changes for Existing Fabric workspace
fix: To reuse Log Analytics across subscriptions
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 and cleans up code and configuration files while updating environment variables and deployment parameters to support EXP changes. Key changes include:
- Removal of the patch for the complete_chat_request in tests and updates to test utilities.
- Updates to dependency management such as the removal of gunicorn and addition (and deduplication) of opentelemetry packages.
- Adjustments in infrastructure deployment files to include new parameters (e.g., existing Log Analytics Workspace) and switch to uvicorn for the web application.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/App/tests/test_app.py | Removed patching for complete_chat_request to streamline test logic. |
| src/App/tests/backend/test_utils.py | Updated the patch target from "app.requests.get" to "backend.utils.requests.get". |
| src/App/requirements.txt | Removed gunicorn dependency and added opentelemetry packages (note duplicate dependency). |
| src/App/requirements-dev.txt | Removed gunicorn dependency to align with production dependency changes. |
| src/App/gunicorn.conf.py | Removed legacy gunicorn configuration to support the switch to uvicorn. |
| src/App/backend/chat_logic_handler.py | Updated environment variable names to reflect new configuration parameters. |
| src/App/WebApp.Dockerfile | Updated startup command from gunicorn to uvicorn with appropriate options. |
| src/App/.env.sample | Updated and standardized environment variable values and defaults. |
| infra/* | Updated deployment scripts and parameters (e.g., Log Analytics Workspace, resource IDs). |
| docs/*.md | Revised deployment and customization documentation to reflect recent changes. |
| .github/dependabot.yml | Removed an outdated pip package configuration section. |
Comments suppressed due to low confidence (3)
src/App/tests/test_app.py:1219
- The patch for complete_chat_request has been removed; please ensure that any workflows relying on this function are adequately covered by the current test setup or are intentionally deprecated.
with patch("app.complete_chat_request", new_callable=AsyncMock) as mock_complete:
src/App/backend/chat_logic_handler.py:20
- Verify that changing the environment variable from 'OPENAI_API_VERSION' to 'AZURE_OPENAI_PREVIEW_API_VERSION' is consistently reflected in all related configurations and documentation.
api_version = os.environ.get("AZURE_OPENAI_PREVIEW_API_VERSION")
infra/deploy_ai_foundry.bicep:58
- [nitpick] Consider adding an inline comment explaining the logic for extracting components from the existing Log Analytics Workspace ID to improve maintainability.
var useExisting = !empty(existingLogAnalyticsWorkspaceId)
* update CustomizingAzdParameters.md * update main.bicepparam * update deployment guide * update key names * Update CustomizingAzdParameters.md * Update main.bicepparam * Update DeploymentGuide.md * update customizingazdparameter file * updates files * update documents --------- Co-authored-by: Shreyas-Microsoft <[email protected]>
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
This pull request introduces several updates across documentation and infrastructure files to enhance deployment flexibility, streamline configurations, and improve support for existing resources. The most significant changes include enabling the reuse of existing Log Analytics workspaces, updating deployment scripts to improve functionality, and removing unused parameters and settings.
Documentation Updates:
uvicorninstead ofgunicornand added a new app setting for port configuration. [1] [2]Infrastructure Enhancements:
Support for Existing Log Analytics Workspace:
existingLogAnalyticsWorkspaceIdparameter ininfra/deploy_ai_foundry.bicepto allow reuse of existing workspaces. Added logic to conditionally use an existing workspace or create a new one. [1] [2] [3] [4]existingLogAnalyticsWorkspaceIdparameter throughinfra/main.bicepandinfra/main.bicepparamfor seamless integration. [1] [2] [3]Application Insights Connection String:
infra/deploy_app_service.bicep. [1] [2]Simplification and Cleanup:
AzureSearchIndexIsPrechunked,AzureOpenAIModelName, andWebAppEnableChatHistoryfrominfra/deploy_app_service.bicep. [1] [2] [3]SCM_DO_BUILD_DURING_DEPLOYMENTandUWSGI_PROCESSESfrom the Azure App Service configuration.These updates collectively improve the flexibility and maintainability of the deployment process while ensuring compatibility with existing resources.
Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information