Skip to content
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

Allow nested computation expressions with custom operations #920

Closed
5 tasks done
absolutejam opened this issue Sep 10, 2020 · 7 comments
Closed
5 tasks done

Allow nested computation expressions with custom operations #920

absolutejam opened this issue Sep 10, 2020 · 7 comments

Comments

@absolutejam
Copy link

absolutejam commented Sep 10, 2020

I propose we allow nested computation expressions, to facilitate a rich builder API.

The existing way of approaching this problem in F# is... There isn't really one. Of course you can use regular old functions and records (See my existing issue about improving the record constructor syntax) but none of them give a really fluid API that is so very nearly possible with computation expressions.

Take the following partial example for building a .gitlab-ci.yml which was nearly there...

let ci =
    gitlabCi {            // GitlabCiBuilder CE
        variables {       // VariablesBuilder CE
            "key1", "val1"
            "key2", "val2"
            "key3", "val3"
            "key4", "val4"
        }

        job "foo" {       // JobBuilder CE
            image "foo"
            stage Build
            script [
                "a"
            ]
        }
    }

This worked fine, with three different builders GitlabCiBuilder, VariablesBuilder and JobBuilder, until I added in a custom operation for the GitlabCiBuilder, and then I got the error This control construct may only be used if the computation expression builder defines a 'For' methodF# Compiler(708).

At first I thought I just didn't implement something correctly (And I'm still not sure if it's just me...), but multiple sources have told me it's not possible to nest CEs and then I found the following closed issue. However, since the links are dead, and the issue was quite a while ago, I was hoping it would be okay to bring this up for discussion again.

Pros and Cons

The advantages of making this adjustment to F# are the ability to provide a clean, flexible builder pattern. It's a construct that reads a lot like a record or json but provides a lot of flexibility under the hood (hidden from the consumer). The likes of Saturn are already adopting a CE for configuraiton, but the limitation of not being ale to nest doesn't do it justice, IMO.

The disadvantages of making this adjustment to F# are... I'm struggling to find any, but I could be biased 😉

Extra information

Estimated cost (XS, S, M, L, XL, XXL): I'm not sure.

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate (This is a duplicate, but with reason!)
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@UnoSD
Copy link

UnoSD commented Sep 15, 2020

This may not be relevant as I haven't tried to implement your example (hence I don't know if you found other blockers), but I do have a working example of nested CE using several Yield to create a custom DSL, see here: https://github.com/UnoSD/Pulumi.FSharp.Extensions the second example in the README.

That said, it is kind of an hack having to use Yield as that means you also allow an equivalent value to be dropped in the nesting CE; this may be beneficial in my use case, but not necessarily in others. Would be nice to have language support for this.

@absolutejam
Copy link
Author

Did you manage to make it work with custom operations too? My issue mostly arises from mixing nested CEs and custom operations.

I could use functions but I feel that one of the benefits of CEs is the custom operations.

@cartermp cartermp changed the title Allow nested computation expressions Allow nested computation expressions with custom operations Sep 20, 2020
@UnoSD
Copy link

UnoSD commented Sep 21, 2020

As discussed on Slack, you can use a Yield overload that accepts a specific type that is the output of another computational expression that then can be just nested and its output will be passed onto the nesting one.

It will look like a custom operation, but in reality it won't be as it is the keyword for the other CE.

working example from my repo:

    let vm =
        windowsVirtualMachine {
            name "PulumiVmName"
            resourceName "AzureNameForVm"
            
            resourceGroup "ResourceGroupName"
            networkInterfaceIds [ config.["nicResourceIdFromConfig"] ]
            size "Standard_D4s_v3"
            
            windowsVirtualMachineOsDisk {
                name config.["vmDiskName"]
                caching "ReadWrite"
                storageAccountType "Standard_LRS"
            }
            
            adminUsername config.["vmUser"]
            adminPassword secret.["encryptedPasswordFromPulumiConfig"]
            
            windowsVirtualMachineSourceImageReference {
                offer "visualstudio2019latest"
                publisher "microsoftvisualstudio"
                sku "vs-2019-comm-latest-win10-n"
                version "latest"
            }
        }

@NickDarvey
Copy link

@UnoSD, I had a look at your repo and the code it generates, but I'm having trouble reapplying it to my own computation expressions. Would you be able to elaborate a little more on how this works?
(I couldn't find any of your messages in the various functional programming Slack workspaces.)

Like @absolutejam, I can define nested computation expression builders just fine, but when I add a custom operations I receive:

This control construct may only be used if the computation expression builder defines a 'For' method

I can see Pulumi.FSharp.Extensions implements For like...

let _combine ((rName, rArgs), (lName, lArgs)) =
    (match lName, rName with
     | null, null -> null
     | null, name -> name
     | name, null -> name
     | _ -> failwith "Duplicate name"),
    (List.concat [ lArgs; rArgs ])

type WindowsVirtualMachineOsDiskBuilder() =
    member _.Yield((_: unit)) = null, [ id ]
    member _.Run((_, args)) =
        List.fold (fun args f -> f args) (WindowsVirtualMachineOsDiskArgs()) args

    member this.Combine(args) = _combine args
    member this.For((args, delayedArgs)) = this.Combine(args, delayedArgs ())
    member _.Delay(f) = f ()
    member _.Zero(_) = ()
    [<CustomOperation("caching")>]
    member _.Caching(((n, args), caching)) =
        n,
        List.Cons(
            (fun (args: WindowsVirtualMachineOsDiskArgs) ->
                args.Caching <- input caching
                args),
            args
        )

but I don't know how to translate that For implementation to a different computation expression builder like this one by @Szer (but with a custom operation added).

// ...
and AllBuilder() =
    member __.Yield (x: FirstLevel list) = x
    member __.Combine (x, y) = x @ y
    member __.Delay f = f()
    member __.Zero() = []
    // Adding this:
    [<CustomOperation "firstDouble">]
    member __.FirstDouble (x, y) = Payload y :: x
// ...
let example =
    all {
        firstDouble 1.
        first { printfn "hello" }
        first {
            2.
            second {
                third { 1 }
                "1"
            }
            10.
        }
    }

(I'm not actually sure how For is being used as I'm not actually using any expressions like { for pattern in expr do cexpr } in example.)

@UnoSD
Copy link

UnoSD commented Oct 30, 2021

@NickDarvey it has been a while, so I do not recall exactly what is going on there, but I think the For is when you use a nested CE (or simply a resulting value) and then you add more operations below.

When this happens, it splits the nesting CE into two and then wants you to combine the results; I think this is to enable syntax similar to list comprehension, hence why it wants the For. If you put the nested CE at the end of the nesting CE, it shouldn't require it (but I'm just 70% sure of that, it has been a while). Hope it helps.

@dsyme
Copy link
Collaborator

dsyme commented Apr 13, 2023

I'm closing out this suggestion as the questions seem to have been answered in the discussions above

@dsyme dsyme closed this as completed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants