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

Support for declarative binding expressions #123

Closed
MisinformedDNA opened this Issue Jan 4, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@MisinformedDNA

MisinformedDNA commented Jan 4, 2018

With normal Azure Functions, I'm able to bind to do some nice declarative work:

    [FunctionName("InjestAndProcessFile")]
    public static async Task Run(
        [BlobTrigger("test/{name}")] Stream input, 
        string name,
        [Blob("archive/0/test/{name}", FileAccess.Write)] Stream output)
    {
        Console.WriteLine($"Archiving {name}...");
        await input.CopyToAsync(output);
    }

However, if I wanted to use Durable Functions, I have to switch an imperative model, which is much noisier:

    [FunctionName("InjestFile")]
    public static async Task Run(
        [BlobTrigger("test/{name}")] Stream input,    // BlobTrigger is required, but the stream is useless here
        string name,
        [OrchestrationClient] DurableOrchestrationClient starter)
    {
        await starter.StartNewAsync("ProcessFile", name);
    }

    [FunctionName("ProcessFile")]
    public static async Task Run(
        [OrchestrationTrigger] DurableOrchestrationContext context,
        IBinder binder)
    {
        var name = context.GetInput<string>();
        using (var input = await binder.BindAsync<Stream>(new BlobAttribute($"test/{name}", FileAccess.Read)))    // For some reason I'm getting an error here: "Cannot access a disposed object"
        using (var output = await binder.BindAsync<Stream>(new BlobAttribute($"archive/0/test/{name}", FileAccess.Write)))
        {
            Console.WriteLine($"Archiving {name}...");
            await input.CopyToAsync(output);
        }
    }

This is just one sample off the top of my head, but you can see how quickly the existing bindings are pushed aside to make way for the Orchestration bindings. Is there anything that can be done to preserve the existing bindings while still using Durable Functions. The bindings are such a great feature of Azure Functions and I'd like to see them continue to shine.

@cgillum

This comment has been minimized.

Show comment
Hide comment
@cgillum

cgillum Jan 4, 2018

Collaborator

I'm not sure this is a valid sample. For one, your orchestrator function is doing I/O which is not allowed (I/O should only be done from activity functions). But more importantly, this example doesn't leverage any of the orchestration capabilities of Durable Functions.

To your point about declarative bindings, your example specifically takes advantage of a feature of blob triggers, which is that you can bind to the name of a blob. With activity triggers in durable functions, there really isn't any metadata that you can bind to since the data being passed around is just raw data from function to function. In that sense, declarative input bindings don't really apply to orchestration scenarios.

Let me know if you have any other thoughts or questions related to this.

Collaborator

cgillum commented Jan 4, 2018

I'm not sure this is a valid sample. For one, your orchestrator function is doing I/O which is not allowed (I/O should only be done from activity functions). But more importantly, this example doesn't leverage any of the orchestration capabilities of Durable Functions.

To your point about declarative bindings, your example specifically takes advantage of a feature of blob triggers, which is that you can bind to the name of a blob. With activity triggers in durable functions, there really isn't any metadata that you can bind to since the data being passed around is just raw data from function to function. In that sense, declarative input bindings don't really apply to orchestration scenarios.

Let me know if you have any other thoughts or questions related to this.

@cgillum cgillum closed this Jan 4, 2018

@cgillum

This comment has been minimized.

Show comment
Hide comment
@cgillum

cgillum Jan 5, 2018

Collaborator

@MikeStall what are your thoughts on this? Do you think there is a way to make binding expressions work in Durable Functions, similar to how the work with queue or blob triggers? It's very possible that I'm missing something.

Collaborator

cgillum commented Jan 5, 2018

@MikeStall what are your thoughts on this? Do you think there is a way to make binding expressions work in Durable Functions, similar to how the work with queue or blob triggers? It's very possible that I'm missing something.

@MikeStall

This comment has been minimized.

Show comment
Hide comment
@MikeStall

MikeStall Jan 5, 2018

@MisinformedDNA - can you update your sample given @cgillum 's feedback? What would you like to be able to write?

While Orchestration's shouldn't have any I/O bindings, Activities could. I think we could reasonably support binding expressions with Activity Triggers - the challenge is just where does the binding data come from? For example, it's be pretty easy to have the activity trigger populate binding data based on it's incoming JObject, similar to how a Queue trigger does for pocos.

MikeStall commented Jan 5, 2018

@MisinformedDNA - can you update your sample given @cgillum 's feedback? What would you like to be able to write?

While Orchestration's shouldn't have any I/O bindings, Activities could. I think we could reasonably support binding expressions with Activity Triggers - the challenge is just where does the binding data come from? For example, it's be pretty easy to have the activity trigger populate binding data based on it's incoming JObject, similar to how a Queue trigger does for pocos.

@MisinformedDNA

This comment has been minimized.

Show comment
Hide comment
@MisinformedDNA

MisinformedDNA Jan 5, 2018

Thanks @cgillum. That explains the runtime errors. (Added issue #124),

Here's a version that compiles

    [FunctionName("InjestFile")]
    public static async Task Run(
        [BlobTrigger("test/{name}")] Stream input,
        string name,
        [OrchestrationClient] DurableOrchestrationClient starter)
    {
        await starter.StartNewAsync("ProcessFile", name);
    }

    [FunctionName("ProcessFile")]
    public static async Task Run(
        [OrchestrationTrigger] DurableOrchestrationContext context)
    {
        var name = context.GetInput<string>();
        await context.CallActivityAsync("CopyFile", name);
    }

    [FunctionName("CopyFile")]
    public static async Task Run2(
        [ActivityTrigger] DurableActivityContext context,
        IBinder binder)
    {
        var name = context.GetInput<string>();
        using (var input = await binder.BindAsync<Stream>(new BlobAttribute($"test/{name}", FileAccess.Read)))
        using (var output = await binder.BindAsync<Stream>(new BlobAttribute($"archive/1/test/{name}", FileAccess.Write)))
        {
            Console.WriteLine($"Archiving {name}...");
            await input.CopyToAsync(output);
        }
    }

And, ideally, I'd like the last function to look like this:

    [FunctionName("CopyFile")]
    public static async Task Run3(
        [ActivityTrigger] DurableActivityContext context,
        string name,    // Derived from context.GetInput<T>()
        [Blob("test/{name}", FileAccess.Read)] Stream input,
        [Blob("archive/1/{name}", FileAccess.Write)] Stream output)
    {
        Console.WriteLine($"Archiving {name}...");
        await input.CopyToAsync(output);
    }

MisinformedDNA commented Jan 5, 2018

Thanks @cgillum. That explains the runtime errors. (Added issue #124),

Here's a version that compiles

    [FunctionName("InjestFile")]
    public static async Task Run(
        [BlobTrigger("test/{name}")] Stream input,
        string name,
        [OrchestrationClient] DurableOrchestrationClient starter)
    {
        await starter.StartNewAsync("ProcessFile", name);
    }

    [FunctionName("ProcessFile")]
    public static async Task Run(
        [OrchestrationTrigger] DurableOrchestrationContext context)
    {
        var name = context.GetInput<string>();
        await context.CallActivityAsync("CopyFile", name);
    }

    [FunctionName("CopyFile")]
    public static async Task Run2(
        [ActivityTrigger] DurableActivityContext context,
        IBinder binder)
    {
        var name = context.GetInput<string>();
        using (var input = await binder.BindAsync<Stream>(new BlobAttribute($"test/{name}", FileAccess.Read)))
        using (var output = await binder.BindAsync<Stream>(new BlobAttribute($"archive/1/test/{name}", FileAccess.Write)))
        {
            Console.WriteLine($"Archiving {name}...");
            await input.CopyToAsync(output);
        }
    }

And, ideally, I'd like the last function to look like this:

    [FunctionName("CopyFile")]
    public static async Task Run3(
        [ActivityTrigger] DurableActivityContext context,
        string name,    // Derived from context.GetInput<T>()
        [Blob("test/{name}", FileAccess.Read)] Stream input,
        [Blob("archive/1/{name}", FileAccess.Write)] Stream output)
    {
        Console.WriteLine($"Archiving {name}...");
        await input.CopyToAsync(output);
    }

@cgillum cgillum reopened this Jan 5, 2018

@cgillum cgillum changed the title from Expanded binding support to Support for declarative binding expressions Jan 5, 2018

@MikeStall

This comment has been minimized.

Show comment
Hide comment
@MikeStall

MikeStall Jan 8, 2018

@cgillum - I think the update would be here:

public IReadOnlyDictionary<string, Type> BindingDataContract => StaticBindingContract;

Instead of a Static Contract, include the Activity's JObject properties in the contract. This is similar to what QueueTrigger does (see https://github.com/Azure/azure-webjobs-sdk/blob/73127679089e244a4fe25fe321bda8114b153878/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingDataProvider.cs )

MikeStall commented Jan 8, 2018

@cgillum - I think the update would be here:

public IReadOnlyDictionary<string, Type> BindingDataContract => StaticBindingContract;

Instead of a Static Contract, include the Activity's JObject properties in the contract. This is similar to what QueueTrigger does (see https://github.com/Azure/azure-webjobs-sdk/blob/73127679089e244a4fe25fe321bda8114b153878/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingDataProvider.cs )

@cgillum cgillum added the enhancement label Jan 26, 2018

@cgillum cgillum added this to the Beta 3 milestone Jan 26, 2018

@cgillum cgillum self-assigned this Feb 27, 2018

@cgillum

This comment has been minimized.

Show comment
Hide comment
@cgillum

cgillum Feb 28, 2018

Collaborator

@MikeStall How do you know what the JObject fields are at function indexing time? I see how I can use reflection to build the contract for POCOs, but I don't see how this can be done for JObject.

Collaborator

cgillum commented Feb 28, 2018

@MikeStall How do you know what the JObject fields are at function indexing time? I see how I can use reflection to build the contract for POCOs, but I don't see how this can be done for JObject.

@MikeStall

This comment has been minimized.

Show comment
Hide comment
@MikeStall

MikeStall Feb 28, 2018

@cgillum - you're right. queue is using a strongly typed POCO and not just a JObject, so it can reflect over the properties. Although we could do something like what HttpTrigger does ... only the top-level properties need to be strongly known, you can use dot expressions {request.query.param1} which can refer into jobjects and be loosely typed. So maybe we can do something like that? The top property would be fixed, give it a generic name like 'data'. So {data} would refer to the JObject, and {data.prop1} would be prop1 on the jobject.

MikeStall commented Feb 28, 2018

@cgillum - you're right. queue is using a strongly typed POCO and not just a JObject, so it can reflect over the properties. Although we could do something like what HttpTrigger does ... only the top-level properties need to be strongly known, you can use dot expressions {request.query.param1} which can refer into jobjects and be loosely typed. So maybe we can do something like that? The top property would be fixed, give it a generic name like 'data'. So {data} would refer to the JObject, and {data.prop1} would be prop1 on the jobject.

@cgillum

This comment has been minimized.

Show comment
Hide comment
@cgillum

cgillum Feb 28, 2018

Collaborator

@MikeStall - The {data} idea is interesting, but if a user tries to bind to {data.prop1}, won't that fail at index time because {data.prop1} isn't defined in the contract?

It seems like HttpTrigger can bind to names defined in the route template, which is nice, but I didn't see how to bind to parameters or payload content that is only known at runtime (reference).

Collaborator

cgillum commented Feb 28, 2018

@MikeStall - The {data} idea is interesting, but if a user tries to bind to {data.prop1}, won't that fail at index time because {data.prop1} isn't defined in the contract?

It seems like HttpTrigger can bind to names defined in the route template, which is nice, but I didn't see how to bind to parameters or payload content that is only known at runtime (reference).

@MikeStall

This comment has been minimized.

Show comment
Hide comment
@MikeStall

MikeStall Feb 28, 2018

@cgillum - In this approach, the contract would define {data} for JObject. This is like what http trigger does. The tests have some examples of this in action: https://github.com/Azure/azure-webjobs-sdk-extensions/blob/60629201329d9b7b07458dcc8b7dc0bc41888754/test/WebJobs.Extensions.Tests/Extensions/Http/HttpTriggerBindingTests.cs#L116

MikeStall commented Feb 28, 2018

@cgillum - In this approach, the contract would define {data} for JObject. This is like what http trigger does. The tests have some examples of this in action: https://github.com/Azure/azure-webjobs-sdk-extensions/blob/60629201329d9b7b07458dcc8b7dc0bc41888754/test/WebJobs.Extensions.Tests/Extensions/Http/HttpTriggerBindingTests.cs#L116

@cgillum

This comment has been minimized.

Show comment
Hide comment
@cgillum

cgillum Feb 28, 2018

Collaborator

@MikeStall Oh - so it just works!

I did what you suggested and simply added data to the contract when I see a JObject parameter type. I was able to successfully bind to a blob named container/{data.Foo} using a JObject as the activity function input containing the text `{"Foo","MyBlobName"}. So it looks like WebJobs implements the dot-resolution for me! I thought i was going to have to do that myself.

Awesome!! I'll send out a PR tomorrow. Thanks for the after-hours help!! :)

Collaborator

cgillum commented Feb 28, 2018

@MikeStall Oh - so it just works!

I did what you suggested and simply added data to the contract when I see a JObject parameter type. I was able to successfully bind to a blob named container/{data.Foo} using a JObject as the activity function input containing the text `{"Foo","MyBlobName"}. So it looks like WebJobs implements the dot-resolution for me! I thought i was going to have to do that myself.

Awesome!! I'll send out a PR tomorrow. Thanks for the after-hours help!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment