Skip to content

Commit 66268c9

Browse files
jongioCopilotCopilot
authored
Fix: Python services use venv instead of system Python (#28)
* Refactor Python service execution to utilize virtual environments - Updated command execution for FastAPI and Streamlit to use Python interpreter from virtual environments instead of directly invoking framework commands (e.g., uvicorn, streamlit). - Modified tests to validate that services correctly detect and use virtual environments. - Added integration tests to ensure proper functionality of Python services with and without virtual environments. - Enhanced error handling in service execution to check for command presence and process setup. - Documented the virtual environment issue and proposed solutions in a new markdown file. * Address PR review feedback: fix test logic and remove artifacts (#29) * Initial plan * Address PR review comments: fix redundant check, remove test artifacts, update .gitignore Co-authored-by: jongio <2163001+jongio@users.noreply.github.com> * Improve test logic clarity in venv_test.go Co-authored-by: jongio <2163001+jongio@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jongio <2163001+jongio@users.noreply.github.com> * Add explicit return statements to Python framework switch cases (#31) * Initial plan * Add explicit return statements to Python framework cases Co-authored-by: jongio <2163001+jongio@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jongio <2163001+jongio@users.noreply.github.com> * Update cli/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update cli/src/internal/service/detector.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Enhance azd context inheritance for child processes and update related documentation - Ensure azd context variables (AZD_SERVER, AZD_ACCESS_TOKEN, AZURE_*) are inherited in service execution and installer commands. - Refactor environment variable handling in service orchestration and command execution. - Update tests to validate changes in environment variable handling. - Add comprehensive documentation on azd context inheritance and its implementation patterns. --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 3c61ad9 commit 66268c9

File tree

14 files changed

+1408
-311
lines changed

14 files changed

+1408
-311
lines changed

‎.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ obj/
7676
.env
7777
.env.local
7878

79+
# Test artifacts - ignore .azure directories outside tests/
80+
.azure/
81+
!tests/**/.azure/
82+
7983
# Test fixtures - preserve lock files and manifests in tests/
8084
!tests/projects/**/package.json
8185
!tests/projects/**/package-lock.json

‎cli/README.md‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -395,33 +395,49 @@ mage dashboarddev # Start dashboard dev server
395395
mage preflight # Run all checks before shipping
396396
```
397397

398-
# Run integration tests (requires external tools)
399-
mage testIntegration
398+
### Running Tests
400399

401-
# Run all tests (unit + integration)
402-
mage testAll
400+
```bash
401+
# Run unit tests only (fast)
402+
go test ./src/internal/service/...
403403

404-
# Run tests with coverage
405-
mage testCoverage
404+
# Run unit tests with verbose output
405+
go test -v ./src/internal/service/...
406406

407-
# Run linter
408-
mage lint
407+
# Run integration tests (requires Python, creates real venvs, slower)
408+
go test -tags=integration -v ./src/internal/service/...
409409

410-
# Format code
411-
mage fmt
410+
# Run all tests via mage
411+
mage test # Unit tests only
412+
mage testIntegration # Integration tests only
413+
mage testAll # Unit + integration tests
414+
mage testCoverage # Tests with coverage report
412415

413-
# Clean build artifacts
414-
mage clean
416+
# Run preflight checks (includes all tests)
417+
mage preflight
418+
```
415419

416-
# Install locally
417-
mage install
420+
**Integration Tests:**
418421

419-
# Run everything (lint, test, build)
420-
mage all
422+
Integration tests create real Python virtual environments and install actual packages. They:
423+
- Verify venv detection works with real filesystems
424+
- Test that Python packages are correctly installed in venvs
425+
- Validate cross-platform path handling (Windows vs Linux/macOS)
426+
- Ensure subprocess spawning inherits the correct environment
421427

422-
# Run preflight checks before committing/releasing
423-
# This includes: dashboard build, formatting, linting, security scan, all tests, and coverage
424-
mage preflight
428+
Requirements:
429+
- Python 3.8+ installed and available in PATH
430+
- Internet connection (for pip install)
431+
- ~2 minutes execution time
432+
433+
**Running specific integration tests:**
434+
435+
```bash
436+
# Run only FastAPI integration test
437+
go test -tags=integration -v ./src/internal/service/... -run TestPythonVenvIntegration/FastAPI
438+
439+
# Run venv fallback test
440+
go test -tags=integration -v ./src/internal/service/... -run TestPythonVenvFallback
425441
```
426442

427443
**Dashboard Development:**

‎cli/docs/dev/azd-context-inheritance.md‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
# AZD Context Inheritance
2+
3+
This document explains how `azd app` ensures that all child processes properly inherit the azd context through environment variables.
4+
5+
## Overview
6+
7+
When running as an `azd` extension, this CLI tool receives azd context through environment variables set by the parent `azd` process. These environment variables must be inherited by all child processes (services, installers, build tools, etc.) to enable:
8+
9+
1. **Azure connectivity**: Access to Azure subscription, resource groups, and deployed resources
10+
2. **Extension framework communication**: gRPC communication with azd via `AZD_SERVER` and `AZD_ACCESS_TOKEN`
11+
3. **Environment values**: All environment-specific configuration from `azd env`
12+
13+
## Key Environment Variables
14+
15+
### Core AZD Extension Variables
16+
17+
- `AZD_SERVER`: gRPC server address for azd communication (e.g., `localhost:12345`)
18+
- `AZD_ACCESS_TOKEN`: JWT token for authenticating with azd extension framework APIs
19+
20+
### Azure Environment Variables
21+
22+
- `AZURE_SUBSCRIPTION_ID`: Current Azure subscription
23+
- `AZURE_RESOURCE_GROUP_NAME`: Target resource group
24+
- `AZURE_ENV_NAME`: Environment name (e.g., dev, staging, prod)
25+
- `AZURE_LOCATION`: Azure region
26+
- `SERVICE_*_URL`: Auto-generated URLs for deployed services
27+
- All other variables from `azd env get-values`
28+
29+
## Implementation Pattern
30+
31+
All code that spawns child processes MUST use one of the following patterns to ensure azd context is inherited:
32+
33+
### Pattern 1: Using executor package (Preferred)
34+
35+
The `executor` package automatically inherits all environment variables:
36+
37+
```go
38+
import "github.com/jongio/azd-app/cli/src/internal/executor"
39+
40+
// All executor functions automatically call cmd.Env = os.Environ()
41+
err := executor.StartCommand(ctx, "dotnet", []string{"run"}, projectDir)
42+
err := executor.RunCommand(ctx, "npm", []string{"install"}, projectDir)
43+
```
44+
45+
### Pattern 2: Direct exec.Command with explicit inheritance
46+
47+
When using `exec.Command` directly, explicitly set `cmd.Env`:
48+
49+
```go
50+
import "os/exec"
51+
52+
cmd := exec.Command("python", "app.py")
53+
cmd.Dir = projectDir
54+
cmd.Env = os.Environ() // REQUIRED: Inherit azd context
55+
cmd.Stdout = os.Stdout
56+
cmd.Stderr = os.Stderr
57+
58+
if err := cmd.Run(); err != nil {
59+
return fmt.Errorf("failed: %w", err)
60+
}
61+
```
62+
63+
### Pattern 3: Service execution with environment merging
64+
65+
Services receive a merged environment that includes azd context. The `OrchestrateServices` function
66+
builds the environment in the following priority order (highest to lowest):
67+
68+
1. **Runtime-specific environment** (from azure.yaml service.env)
69+
2. **Custom .env file variables** (from --env-file flag)
70+
3. **OS environment** (includes all azd context: AZD_*, AZURE_*)
71+
72+
```go
73+
// In OrchestrateServices, each service receives:
74+
// 1. Start with os.Environ() - includes azd context
75+
serviceEnv := make(map[string]string)
76+
for _, e := range os.Environ() {
77+
pair := strings.SplitN(e, "=", 2)
78+
if len(pair) == 2 {
79+
serviceEnv[pair[0]] = pair[1]
80+
}
81+
}
82+
// 2. Merge custom --env-file variables
83+
for k, v := range envVars {
84+
serviceEnv[k] = v
85+
}
86+
// 3. Merge service-specific variables (highest priority)
87+
for k, v := range rt.Env {
88+
serviceEnv[k] = v
89+
}
90+
91+
// createServiceCommand converts the map to environment slice
92+
process, err := service.StartService(runtime, serviceEnv, projectDir)
93+
```
94+
95+
## Files Implementing Azd Context Inheritance
96+
97+
### Service Execution
98+
99+
- **`cli/src/internal/service/orchestrator.go`**
100+
- `OrchestrateServices()`: Builds environment for each service starting with `os.Environ()` to inherit azd context
101+
- Merges custom env vars from --env-file and runtime-specific variables
102+
- Ensures all services receive AZD_*, AZURE_* variables
103+
104+
- **`cli/src/internal/service/executor.go`**
105+
- `createServiceCommand()`: Creates commands with merged environment that includes azd context
106+
- All service processes receive full azd environment
107+
108+
- **`cli/src/internal/service/env.go`**
109+
- `ResolveEnvironment()`: Merges environment variables starting with `os.Environ()` which includes azd context
110+
- Preserves `AZD_*` and `AZURE_*` variables through all merging operations
111+
112+
### Dependency Installation
113+
114+
- **`cli/src/internal/installer/installer.go`**
115+
- All `exec.Command` calls include `cmd.Env = os.Environ()`
116+
- Ensures dependency installers (npm, pip, poetry, uv, dotnet) have azd context
117+
118+
### Command Execution
119+
120+
- **`cli/src/internal/executor/executor.go`**
121+
- `RunCommand()`: Sets `cmd.Env = os.Environ()` for all commands
122+
- `StartCommand()`: Sets `cmd.Env = os.Environ()` for long-running processes
123+
- `StartCommandWithOutputMonitoring()`: Sets `cmd.Env = os.Environ()` with output capture
124+
125+
### Runners
126+
127+
- **`cli/src/internal/runner/runner.go`**
128+
- All runner functions use the `executor` package which handles environment inheritance
129+
- `RunAspire()`, `RunNode()`, `RunPython()`, `RunDotnet()` all inherit azd context
130+
131+
## Verification
132+
133+
To verify azd context is properly inherited, you can:
134+
135+
1. **Check environment in your application**:
136+
```python
137+
# Python example
138+
import os
139+
print(f"AZD_SERVER: {os.getenv('AZD_SERVER')}")
140+
print(f"AZURE_SUBSCRIPTION_ID: {os.getenv('AZURE_SUBSCRIPTION_ID')}")
141+
```
142+
143+
2. **Enable verbose logging**:
144+
```bash
145+
azd app run --verbose
146+
```
147+
148+
3. **Check test files**:
149+
- `cli/tests/projects/aspire-test/TestAppHost/AppHost.cs` demonstrates azd context verification
150+
151+
## Common Pitfalls
152+
153+
### ❌ DON'T: Create empty environment
154+
155+
```go
156+
// This loses azd context!
157+
cmd := exec.Command("python", "app.py")
158+
cmd.Env = []string{} // WRONG: No azd context
159+
```
160+
161+
### ❌ DON'T: Forget to set Env
162+
163+
```go
164+
// This uses a default environment that may not include azd context
165+
cmd := exec.Command("python", "app.py")
166+
// Missing: cmd.Env = os.Environ()
167+
cmd.Run()
168+
```
169+
170+
### ✅ DO: Always inherit from os.Environ()
171+
172+
```go
173+
// Correct: Inherit azd context
174+
cmd := exec.Command("python", "app.py")
175+
cmd.Env = os.Environ()
176+
cmd.Run()
177+
```
178+
179+
### ✅ DO: Use executor package when possible
180+
181+
```go
182+
// Best: Use executor which handles environment automatically
183+
executor.StartCommand(ctx, "python", []string{"app.py"}, dir)
184+
```
185+
186+
## Testing
187+
188+
When writing tests that spawn processes, consider:
189+
190+
1. Set up test environment variables for `AZD_*` and `AZURE_*` keys
191+
2. Use `t.Setenv()` in Go tests to set environment variables
192+
3. Verify child processes receive expected environment variables
193+
194+
## Reference
195+
196+
For more information about the azd extension framework, see:
197+
- [Azure Developer CLI Extension Framework Documentation](https://github.com/Azure/azure-dev/blob/main/cli/azd/docs/extension-framework.md)
198+
- [Environment Service gRPC API](https://github.com/Azure/azure-dev/blob/main/cli/azd/grpc/proto/environment.proto)

0 commit comments

Comments
 (0)