F# friendly / idiomatic API suggestions

mikhailshilkov commented 6 years ago

I have several random thoughts about how F# code for Durable Functions could be improved. They are all simple things, no need to re-imagine the whole framework.

The goals of this issue are a) Start discussion and gather ideas from F# folks b) See if that generally fits into Durable Functions vision c) Ideally, design a way to go forward (define F# extension library?)

If that works out, I would be willing to contribute to further design and implementation.

CallActivityAsync

Activities and Orchestrator are always in the same project, so it just feels wrong to call activity with untyped method

let! isClear = context.CallActivityAsync<bool>("E3_GetIsClear", input.Location)

I would prefer to make it typed, something like

let getIsClearActivity = Activity.of context GetIsClear
let! isClear = getIsClearActivity input.Location

Context

We'd probably need a separate Context type to provide F#-specific methods on it. We can just convert it

let fsharpContext = makeFsharpContext context

but ideally it could replace the existing type in function declaration

let Run([<OrchestrationTrigger>] fsharpContext: DurableFsOrchestrationContext) = task {

Input Values

Inputs return nulls if input is not provided, even for F# record types which are in theory not-nullable. That's confusing. It should return Option<T> when trying to get Input parameters.

Now:

let Run([<OrchestrationTrigger>] backupContext: DurableOrchestrationContext) = task {
    let input = backupContext.GetInput<string>()
    let rootDirectory = 
      if String.IsNullOrEmpty(input) then input.Trim()
      else Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName

Could be:

let Run([<OrchestrationTrigger>] backupContext: DurableFsOrchestrationContext) = task {
    let input = backupContext.GetInput<string>() // Option<string>
    let rootDirectory = 
      match input with
      | Some s -> s.Trim()
      | None -> Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName

We can then replace the super ugly check for null in Monitor example

let input = monitorContext.GetInput<MonitorRequest>()
if (obj.ReferenceEquals (input, null)) then
  failwith "An input object is required."

with a simple pattern match as above.

Now, in ideal case, input parameters should be... just input parameters:

let Run([<OrchestrationTrigger>] context: DurableFsOrchestrationContext, request:  MonitorRequest) =

It could apply to C# too, not sure if there was a good reason for not doing so.

Error handling

Same story about exceptions: F# doesn't like exceptions.

Every activity call could return Result<T, E>, so that the client code can and should handle the error case properly. Say we have

let! hello1 = context.CallActivityAsync<string>("E1_SayHello", "Tokyo")
let! hello2 = context.CallActivityAsync<string>("E1_SayHello", "Seattle")
let! hello3 = context.CallActivityAsync<string>("E1_SayHello", "London")

Any error handling other than covering all with try-catch is difficult. But with Result we could do pattern match

match (hello1, hello2, hello3) with
| Ok h1, Ok h2, Ok h3 -> do stuff
| Ok h1, _, _ -> at least we had h1!
| _, _, _ -> and so on

It might make sense to make a separate method for that like TryCallActivity

Currying

CallActivity should probably be curried to simplify partial application.

Now:

let tasks = files |> Array.map (fun f -> context.CallActivityAsync<int64>("E2_CopyFileToBlob", f))

Could be:

let tasks = files |> Array.map (context.CallActivityAsync<int64> "E2_CopyFileToBlob"))

Building blocks: Timeouts

It's not normal to write code like this in F#:

use timeoutCts = new CancellationTokenSource()

// The user has 90 seconds to respond with the code they received in the SMS message.
let expiration = context.CurrentUtcDateTime.AddSeconds 90.
let timeoutTask = context.CreateTimer(expiration, timeoutCts.Token)

let challengeResponseTask = context.WaitForExternalEvent<int>("SmsChallengeResponse")
let! winner = Task.WhenAny(challengeResponseTask, timeoutTask)
let authorized =
if (winner = timeoutTask) then false
else challengeResponseTask.Result = challengeCode

// All pending timers must be complete or canceled before the function exits.
if (not timeoutTask.IsCompleted) then timeoutCts.Cancel()

return authorized

Instead, there should be a primitive on top, e.g. Orchestrator.withTimeout which would wrap the original flow and insert timeout mechanics around:

task {
  let! challengeCode = context.CallActivityAsync<int>("E4_SendSmsChallenge", phoneNumber)

  let! challengeResponseTask = context.WaitForExternalEvent<int>("SmsChallengeResponse")
  return challengeResponseTask.Result = challengeCode
}
|> Orchestrator.withTimeout 90.

Tasks can be nested, so withTimeout could be used inside a bigger orchestration if needed.

Building blocks: Retries

Same applies to retry loops. While loops are possible in F#, and I used recursion in samples, non of those is particularly readable. Instead of doing some

let rec challenge i = task {
  let! challengeResponseTask = context.WaitForExternalEvent<int>("SmsChallengeResponse")
  if (challengeResponseTask = challengeCode) then return true
  elif i >= 0 then return! challenge (i - 1)
  else return false
}

let! authorized = challenge 3

we could make this work:

let! authorized =  
  context.WaitForExternalEvent<int>("SmsChallengeResponse")
  |>  Orchestrator.retryUntil (fun r i -> r = challengeCode || i >= 3)

The building blocks could probably keep growing organically over time as the need arises.

I invite anybody with F# and Durable Functions experience to comment on this case.

cgillum commented 6 years ago

Thanks for adding these suggestions. I like the idea of making Durable Functions more friendly to F#.

You mentioned this briefly, but especially where we look at things like Option<T>, do you think it would make sense to have a separate F# extension library, like Microsoft.Azure.WebJobs.Extensions.DurableTask.FSharp which brings in those new APIs? I feel like this would be a nice clean way to introduce anything that would be specific to F#. It would have a dependency on Microsoft.Azure.WebJobs.Extensions.DurableTask and could therefore be the only package dependency that F# devs need to pull in explicitly.

mikhailshilkov commented 6 years ago

@cgillum Yes, that was my thought process too

mikhailshilkov commented 6 years ago

I got a chance to play with my ideas, so I have a little prototype working.

Activity is a combination of name and function (name is then used in orchestrator to call it):

let sayHello = Activity.define "E1_SayHello" (sprintf "Hello %s!")

Every good F# API has to use ~~monads~~ computation expressions, so I created orchestrator CE:

let helloSequence = orchestrator {
    let! hello1 = callActivity sayHello "Tokyo"
    let! hello2 = callActivity sayHello "Seattle"
    let! hello3 = callActivity sayHello "London"

    return [hello1; hello2; hello3]
}

Basically, it takes care of the context implicitly, while providing friendly callActivity API. Note that callActivity is low ceremony but strongly typed: it infers types from activity definition.

The not-so-nice part is that I still need to keep Function definitions for Functions runtime to be able to generation bindings and entry points:

[<FunctionName("E1_SayHello")>]
let SayHello([<ActivityTrigger>] name) = sayHello.apply name

[<FunctionName("E1_HelloSequence")>]
let Run([<OrchestrationTrigger>] context: DurableOrchestrationContext) = helloSequence context

Any hints on how to get rid of those are very welcome. They are 100% mechanical, maybe ok for now, but would be nice to generate them behind the scenes in the end.

Opinions? Is this direction worth pursuing?

mikhailshilkov commented 6 years ago

Orchestrator with optional input value and parallel activity calls:

let backup (inputDirectory: string option) = orchestrator {
  let rootDirectory =
    match inputDirectory with
    | Some directory -> directory.Trim()
    | None -> Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName

  let! files = Activity.call getFileList rootDirectory
  let! totalBytes = Activity.all copyFileToBlob files
  return Array.sum totalBytes
}

[<FunctionName("E2_BackupSiteContent")>]
let Run([<OrchestrationTrigger>] context: DurableOrchestrationContext) = 
  Orchestrator.run context backup

cgillum commented 6 years ago

To answer your previous question:

Opinions? Is this direction worth pursuing?

I think this is worth pursuing. I feel pretty strongly that we should be very intentional in our support of languages beyond C#, and this is a great example of how we can move towards making F# a first-class citizen of the Durable Functions family (and in some ways leading the way in terms of innovation or ease of use). I also expect it will help motivate further investment in F# overall for Azure Functions.

mikhailshilkov commented 6 years ago

Phone verification workflow:

let verifyPhone (phoneNumber: string) = 
    orchestrator {
        let! challengeCode = Activity.call generateChallenge phoneNumber
        let! response = Activity.waitForEvent "SmsChallengeResponse"
        return response = challengeCode
    }
    |> Orchestrator.timeout 90.
    |> Orchestrator.retry 3

[<FunctionName("E4_SmsPhoneVerification")>]
let Run([<OrchestrationTrigger>] context: DurableOrchestrationContext) = 
    Orchestrator.run context verifyPhone

gsomix commented 6 years ago

@mikhailshilkov Looks good! I'm trying to compare it with C# example and I have some questions:

Where is validation of phoneNumber? What happens if phoneNumber is null?
In the example we want to retry only response receiving. But it seems in your example whole orchestrator block will be repeated (with sending challenge code), right?

mikhailshilkov commented 6 years ago

@gsomix Thanks for having a look at this!

It's inside Orchestrator.run and (similar to the original example) it will throw exception if input is not provided

let run (context: DurableOrchestrationContext) f =
      let input = context.GetInput<string>()
      if String.IsNullOrEmpty(input) then raise (ArgumentNullException("input", "Input is required."))
      else f input context

Ah, right, my mistake, should be something like this then:

let verifySecurityCode (challengeCode: int) =
    orchestrator {
        let! response = Activity.waitForEvent "SmsChallengeResponse"
        return response = challengeCode
    }
    |> Orchestrator.retry 3

let verifyPhone (phoneNumber: string) = 
    orchestrator {
        let! challengeCode = Activity.call generateChallenge phoneNumber
        return! verifySecurityCode challengeCode
    }
    |> Orchestrator.timeout 90.

cgillum commented 6 years ago

Hey @mikhailshilkov just wanted to check in with you on this. Are you still interested in contributing it? I would love to have it. I think it would really make F# a first-class citizen of Durable Functions and I would happily give it proper first-class treatment in our docs.

mikhailshilkov commented 6 years ago

@cgillum Yes.

The problem is that I switched projects a bit, so I'm currently not using durable functions, and it's hard to find time for that. I really want to get this done though, so I guess I should just start a PR and get this going.

Any particular time frame that this would fit nicely?