MicrosoftDocs / azure-docs

Open source documentation of Microsoft Azure
https://docs.microsoft.com/azure
Creative Commons Attribution 4.0 International
10.24k stars 21.41k forks source link

Python Function Chaining Example Incorrect Example #77633

Open tyler555g opened 3 years ago

tyler555g commented 3 years ago

Python example does not show true function chaining. Only shows how to call one function with different parameters.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

cgillum commented 3 years ago

Pinging @davidmrdavid

davidmrdavid commented 3 years ago

Thanks for reaching out @tyler555g!

Ah yes, this is an interesting one and I see where you're coming from. While the Python example doesn't really take the output of one activity and pass it as the input to the next, it can still be considered "function chaining" because the pattern, in the docs themselves, is defined as follows:

Function chaining refers to the pattern of executing a sequence of functions in a particular order. Often the output of one function needs to be applied to the input of another function.

In other words, the pattern's main characteristic is to establish a list of Functions that need to be called in order. While it's common to do this in a way that takes the output of the previous Function and applies it to the next one in the sequence, this is not strictly necessary.

Also, the Python example is equivalent to their JS and C# counterparts, so it's been written to conform to those examples :) . Please let me know if this is unclear, but I believe the documentation is currently correct as it stands.

That being said, I do wonder if we should re-think these snippets and have it show the outputs of activities become the inputs to the next in the sequence. @cgillum, @ConnorMcMahon, what do y'all think? That seems to me to be a little more interesting to show.

tyler555g commented 3 years ago
import azure.functions as func
import azure.durable_functions as df

def orchestrator_function(context: df.DurableOrchestrationContext):

    data = context.get_input()
    result1 = yield context.call_activity("Activity1", data)
    context.set_custom_status(result1)
    result2 = yield context.call_activity("Activity2", result1)
    context.set_custom_status(result2)
    result3 = yield context.call_activity("Activity3", result2)
    result4 = yield context.call_activity("Activity4", result2)
    return [result3, result4]

main = df.Orchestrator.create(orchestrator_function)

As I have since figured this out. Leaving my suggested example here to help others trying to figure this out.

tyler555g commented 3 years ago

I think a compromise similar to what I have done above shows both types and provides clarity for those trying to understand the correct pattern. Thoughts @davidmrdavid , @cgillum , @ConnorMcMahon ?

davidmrdavid commented 3 years ago

Thank you for the proposal, @tyler555g!

Is there a reason why you included the set_custom_status APIs in the snippet? I'm trying to understand if that's meant to communicate something about the pattern. In general, I would prefer to use only the call_activity APIs as that would keep it simpler and easier to parse.

I personally like the following compromise:

import azure.functions as func
import azure.durable_functions as df

def orchestrator_function(context: df.DurableOrchestrationContext):
    data = context.get_input()
    result1 = yield context.call_activity("Activity1", data)
    result2 = yield context.call_activity("Activity2", result1)
    result3 = yield context.call_activity("Activity3", result2)
    return [result1, result2, result3]

main = df.Orchestrator.create(orchestrator_function)

That being said, as I mentioned, I don't believe that function-chaining necessitates passing the output of the previous function as input to the next. I think it's sufficient to simply call 3 functions in a sequence, one after the other. As a result, I'm biased to keep the sample as-is, as I believe it currently communicates the minimum requirement for function-chaining: functions being invoked in a sequence :)

ConnorMcMahon commented 3 years ago

I'm not opposed to explicitly having the second example, separated by an explanation along the lines of:

"Because we are scheduling these activities sequentially, we can utilize the return value of any of these functions in any subsequent activity executions."

My only other critique would be that I feel that when you pass sequential results to each other like you do in that example, generally you don't care about the intermediate results like result1 and result2 and really just care about the final result, so maybe just return result3?

davidmrdavid commented 3 years ago

Just returning result3 works for me!