Skip to content

Conversation

@pamelafox
Copy link
Contributor

Purpose

This pull request updates the Azure deployment workflow with an automated smoke test to validate the deployed chat endpoint. The "Smoke Test Chat Endpoint" step automatically checks the /chat/stream endpoint of the deployed service, retrying up to 6 times to confirm it responds with HTTP 200, improving early detection of deployment issues.

Does this introduce a breaking change?

[ ] Yes
[X] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Watch the workflow

@pamelafox pamelafox requested a review from Copilot October 1, 2025 23:50
Copy link

Copilot AI left a 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 pull request enhances the Azure deployment workflow by adding automated smoke tests to validate the deployed service. The changes include refactoring the OpenAI client configuration for better maintainability and implementing a comprehensive testing pipeline.

  • Refactored OpenAI client configuration with cleaner imports and improved error handling
  • Added comprehensive smoke tests including basic HTTP checks and end-to-end Playwright tests
  • Updated dependency specification for OpenAI package version pinning

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/requirements.txt Pins OpenAI version to exact match for dependency stability
src/quartapp/chat.py Refactors OpenAI client configuration with cleaner imports and improved logging
scripts/e2e_chat_playwright.py Adds new end-to-end browser test using Playwright for chat functionality validation
.github/workflows/azure-dev.yaml Extends workflow with smoke test job including basic HTTP checks and Playwright tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

build:
runs-on: ubuntu-latest
outputs:
uri: ${{ steps.output.outputs.uri }}
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: the uri output should be indented with 6 spaces to align with the outputs: key, not 8 spaces.

Suggested change
uri: ${{ steps.output.outputs.uri }}
uri: ${{ steps.output.outputs.uri }}

Copilot uses AI. Check for mistakes.
)
current_app.logger.info("Using Azure OpenAI with key")
elif os.getenv("RUNNING_IN_PRODUCTION"):
client_id = os.environ["AZURE_CLIENT_ID"]
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using os.environ directly will raise a KeyError if the environment variable is missing. Consider using os.getenv() with a descriptive error message or add proper error handling to provide clearer feedback when the required environment variable is not set.

Suggested change
client_id = os.environ["AZURE_CLIENT_ID"]
client_id = os.getenv("AZURE_CLIENT_ID")
if not client_id:
raise RuntimeError("AZURE_CLIENT_ID environment variable is not set but is required for managed identity authentication.")

Copilot uses AI. Check for mistakes.
base_url=os.environ["AZURE_OPENAI_ENDPOINT"] + "/openai/v1/",
api_key=token_provider,
)
tenant_id = os.environ["AZURE_TENANT_ID"]
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using os.environ directly will raise a KeyError if the environment variable is missing. Consider using os.getenv() with a descriptive error message or add proper error handling to provide clearer feedback when the required environment variable is not set.

Suggested change
tenant_id = os.environ["AZURE_TENANT_ID"]
tenant_id = os.getenv("AZURE_TENANT_ID")
if tenant_id is None:
raise RuntimeError("AZURE_TENANT_ID environment variable is not set. Please set it to continue.")

Copilot uses AI. Check for mistakes.
@pamelafox pamelafox merged commit 3eb2d0a into main Oct 1, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants