-
Notifications
You must be signed in to change notification settings - Fork 173
Clarify Option-based approach for conditional values in arm builder #1231
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
base: master
Are you sure you want to change the base?
Conversation
- Added Zero, Combine, Delay, Run, and For methods to DeploymentBuilder - Enables control flow constructs for flexible deployment composition - Leverages existing Option type overloads on output method - Added comprehensive tests demonstrating Option-based conditional outputs - Supports copy-and-update pattern for conditional deployment composition Co-authored-by: ninjarobot <[email protected]>
- Created EmptyState() helper method in DeploymentBuilder - Both Yield() and Zero() now use this shared method - Improves maintainability by eliminating duplicate initialization code Co-authored-by: ninjarobot <[email protected]>
|
@copilot how does your solution enable either of these? // or |
My solution does NOT enable those exact patterns due to F# limitations. Custom operations (like The methods I added (Zero, Combine, etc.) enable control flow for NON-custom operations, but The actual solution uses the existing Option overloads on the arm {
location Location.UKSouth
add_resource myVm
output "vmIP" (myVm.PublicIpId |> Option.map (fun ip -> ip.ArmExpression))
}This achieves the same goal - conditional outputs based on Option values - without violating F# constraints. The Option mapping happens outside the custom operation, which is allowed. I should clarify the PR description to avoid confusion about what patterns are possible. Commits: 4af8de3, 317dabf |
- Added comments explaining F# limitation: custom operations cannot be used inside control flow - Clarified that Zero/Combine/For enable control flow for non-custom operations only - Helps prevent confusion about what patterns are possible (addresses PR feedback) Co-authored-by: ninjarobot <[email protected]>
|
You can use conditionals in seq expressions, async expressions, backgroundTask expressions. This doesn't seem like a language limitation. What's the issue here @copilot ? |
You're correct - the distinction is important. seq/async/backgroundTask use Farmer builders use The limitation is: custom operations cannot be used with if/match/for/while/try, not computation expressions in general. Since Farmer's primary operations are custom operations, this affects most Farmer builder scenarios. The Option-based solution works because the Option mapping happens outside the custom operation call, passing the result to the custom operation. |
- Updated comments to specify custom operations are the issue, not CEs in general - Distinguished from seq/async which use yield/let! (not custom operations) - Custom operations (marked with [<CustomOperation>]) cannot be used in control flow - Addresses feedback that seq/async/backgroundTask DO support control flow Co-authored-by: ninjarobot <[email protected]>
Users cannot conditionally include outputs based on optional resource properties (e.g., VM PublicIpId) without creating multiple builder copies or using copy-and-update workarounds.
Changes
DeploymentBuilder enhancements:
Zero(),Combine(),Delay(),Run(), andFor()methods for future CE extensibilityEmptyState()helper to eliminate duplication betweenYield()andZero()[<CustomOperation>]) cannot be used in control flow, distinct from seq/async which useyield/let!(not custom operations)Tests:
outputmethod (already existed, now formally tested)Solution
The
outputmethod already acceptsstring optionandArmExpression option, handling None cases by omitting the output. Users can write:When
PublicIpIdisNone, the output is omitted. WhenSome, the ARM expression is added.Important: What This Enables
❌ Does NOT Enable
The control flow patterns requested in the original issue are NOT possible due to F# compiler error FS3086:
Why? F# prohibits custom operations (marked with
[<CustomOperation>]) inside control flow constructs. This is different from seq/async/backgroundTask which useyield/let!/return(NOT custom operations). Farmer'soutput,add_resource,locationetc. are custom operations, so they cannot be used insideif,match,for,while, ortry.✅ Does Enable
The Zero/Combine/For methods provide CE infrastructure, but custom operations still cannot be used inside control flow. The practical solution uses Option type overloads (shown above) or copy-and-update pattern:
Technical Details
The distinction is:
seq/async/backgroundTask: Use
yield,let!,return→ NOT custom operations → control flow worksFarmer builders: Use
output,add_resource,location→ ARE custom operations → control flow prohibited (FS3086)Fixes Conditional values in Farmer builders, or composing builders? #731
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.