Allow project mappings to preserve environment templates - #9897
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed changes are well-covered, with no unresolved approval-blocking issues.
Review tier: Balanced
Findings: None
What changed in this PR
Adds context-aware project mappings so environment templates remain intact during RPC/model round trips.
Changes:
- Adds mapper context and environment-substitution controls.
- Exposes unexpanded strings and map values.
- Adds coverage for context propagation and template-preserving round trips.
| File | Description |
|---|---|
cli/azd/pkg/project/mapper_registry.go |
Applies substitution settings across project mappings. |
cli/azd/pkg/project/mapper_registry_test.go |
Tests template-preserving project conversions. |
cli/azd/pkg/osutil/expandable_string.go |
Exposes unexpanded string templates. |
cli/azd/pkg/osutil/expandable_map.go |
Exposes unexpanded map values. |
cli/azd/pkg/osutil/expandable_map_test.go |
Tests raw expandable-map behavior. |
cli/azd/internal/mapper/mapper.go |
Adds mapper context and substitution configuration. |
cli/azd/internal/mapper/mapper_test.go |
Tests context propagation and configuration isolation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…e coverage up, so this is just arbitrary to improve the CC gate.
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Nested package-level conversions still discard the supplied context.
Review tier: Balanced
Findings: None
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/pkg/project/mapper_registry.go:135
- Context propagation remains inconsistent: this path and the project/service paths use
WithContext(ctx), but the same registry still has nested package-levelmapper.Convertcalls for artifact collections, service results, and service contexts (for example, lines 80, 229, 496, and 612). Those calls revert to the background context, so cancellation and any converter-specific values set through this new general-purpose API are silently lost below those nodes. Propagatectxthrough every nested conversion, or centralize nested conversion so handlers cannot accidentally drop it.
…subst on and off (which is allowed, per call)
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
5455d41
into
Azure:main
The mapper registrations for the service-related RPC types were automatically doing envsubst. For the round-tripping we want to do for layers, we want to be able to cleanly convert between RPC and our models, without any additional translation.
This PR makes it so you can set your own context, with a mapper, which will then flow naturally into each registration handler. The checking and handling of the envsubst stuff still has to be done for each field, but it's a fairly easy contract to maintain, and there are helpers.